Re: undefined behavior in builtin/am.c

From: Jeff King <peff@peff.net>
Date: Jul 1, 2022, 17:53 UTC
Message-ID: <Yr80s8VP8ECCXKd+@coredump.intra.peff.net>
In-Reply-To: <130c3636-b978-1600-df53-6a38c3414a88@gmx.de>

On Fri, Jul 01, 2022 at 07:03:18PM +0200, Roland Illig wrote:

Show 13 quoted lines

> The macro 'isspace' must only be called with an integer representable as
> an 'unsigned char', or with the value of the macro EOF.
> 
> On platforms where plain 'char' is a signed integer type, any character
> whose value is negative invokes undefined behavior (except for the one
> character that by coincidence has the same value as the macro EOF).
> 
> To fix this, write '!isspace((unsigned char)*str)' instead.
> 
> I have no idea how to trigger this part of the code but for someone who
> knows this part of Git, it should be easy. Depending on the platform,
> this kind of error may be silently ignored or crash the program, as
> always with undefined behavior.

We don't use the system isspace(), but instead our own macro wrappers in git-compat-util.h. They do the cast to unsigned char themselves.

I won't be surprised if re-defining a system name as a macro is also technically undefined behavior, but I don't think we've found a system that has a problem with it in the past 17 years. :)

-Peff