Date: Tue, 2 May 2023 20:00:45 -0400 From: Rich Felker <dalias@...c.org> To: Jₑₙₛ Gustedt <jens.gustedt@...ia.fr> Cc: musl@...ts.openwall.com Subject: Re: patches for C23 On Tue, May 02, 2023 at 07:20:09PM -0400, Rich Felker wrote: > On Tue, May 02, 2023 at 03:59:03PM +0200, Jₑₙₛ Gustedt wrote: > > on Tue, 2 May 2023 08:57:40 +0200 you (Jₑₙₛ Gustedt > > <jens.gustedt@...ia.fr>) wrote: > > > > > I'll also setup a git repository for those who would be willing to > > > test the whole. Just be aware that is really testing and review, not > > > yet ready for direct inclusion. So probably this will be rebased > > > several times. > > > > So this can now be found here > > > > https://icube-forge.unistra.fr/icps/musl/ > > > > with my additional branch called "c23". I also put on tags for what I > > think might be good groups to treat together. An overview should be > > accessible here > > > > https://icube-forge.unistra.fr/icps/musl/-/network/c23?ref_type=heads > > > > Let me know if you have any problems in accessing this. > > > > I will then post the patches on the ML later, probably need some time > > for that to do it right. > > One quick find, in > https://icube-forge.unistra.fr/icps/musl/-/commit/3a2b83bf32d7c94f1bf0b2b2fd6ba8b6bf980d99 > > - np = strtoul(r+9, &z, 10); > + np = strtoul(r+9, (char**)&z, 10); > > is UB. Accessing a const char * as char *. I would prefer in general > we just #undef any of the const-stuff-tg macros in files that use > them, or just have src/include/string.h always do that. (Not really > needed since musl source is written in c99 not c23, but it would be > nice to have it also compile with c11 and c23 compilers, so I think > the #undef is useful.) Some more things I noticed, some of them general: For compiler feature stuff like __has_c_attribute, this should not be littered all over headers. We have features.h that already abstracts getting the ones we need. If there are others strictly needed, those should be abstracted there too. It's probably not needed to avoid exposing new functions under older __STDC_VERSION. We generally do not aim for strict namespace conformance to older versions of the standards. On the flip side, it's not needed to jump through compiler-specific hoops to get new features that can't be obtained in standard ways without c23 mode. For example, nullptr_t does not need clang special cases. If it's not c23, it just doesn't need to be defined (and in fact strictly speaking it's a namespace violation to define it, but as above we don't really care about that.) It's not needed to make namespace-safe internal __-prefixed versions of functions unless they're used to implement functions in a more restrictive namespace profile. For example, POSIX functions needed to implement STDC functions need this treatment, but STDC functions basically never do. Language/compiler baseline for building musl is not going to go up, so this complicates some things, especially implementing the int128 stuff. This will need pop_arg to call out to an arch-provided asm function that bypasses the C type system to get the nonexistent-type argument off the va_list and store it in a pair of uint64_t. There are not going to be different versions of scanf/strto* because there's just no way to do that in a conforming way (the standard allows declaring these functions yourself without including the header, which would not get the remapping). As above, strict conformance to outdated versions of the standard is just not a priority. musl's claim/target is conformance to current versions only and sometimes, on a case-by-case basis, partial support for older ones. (Looking at it again, I don't understand how the code in your repo was actually intended to provide different versions. __intscan, etc. are not public interface boundaries and references to them never appear in applications, only internally within libc. Your code seems to be conditioning which gets used based on the STDC version in use for building musl, which is completely decoupled from the version of the standard that a given application is being built/linked for.)
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.