On Thu, Dec 15, 2022 at 11:07:24AM +0100, Ævar Arnfjörð Bjarmason wrote: > Excluding compat/ the diff is the below. Some of it's broken, but some > of it suggests things that are worth picking up, e.g. the return value > of xmalloc() being cast (a C++-ism), "(char *)NULL" (do we ever need to > cast NULL?) Yeah, getting rid of explicit casts to/from void (as in xmalloc()) is a good thing, IMHO. Casting NULL is trickier. In normal assignment, no, it should never be needed. When passed to a variadic function (which covers all the cases in your patch below), the compiler needs to know that it's a pointer, and not the integer 0. So of the two allowed null pointer constants, only one is guaranteed to work: execl(foo, 0); /* bad! there's nothing to say it's a pointer */ execl(foo, (char *)0); /* ok */ though whether it matters in practice depends on your ABI, I think. But what about NULL? My copy of C99 (7.17.3) says it "expands to an implementation-defined null pointer constant". So it could be either of those. And that is backed up by looking at "git log -S')NULL'", which yields 5d314759d7 (Cast execl*() NULL sentinels to (char *), 2010-07-24). That said, I think we have lots of bare NULLs passed to variadic functions. Any function with LAST_ARG_MUST_BE_NULL will have a NULL in each of its callers, and we do not bother casting most of them. So I think this is one of those "technically could violate the standard, but OK in practice" things. -Peff
