Re: [PATCH] git: remove unneeded casts

[Jeff King <peff@peff.net>]

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