Re: undefined behavior in builtin/am.c

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Jul 1, 2022, 17:54 UTC
Message-ID: <d717324b-d7dc-fc5c-7bd7-2b74c4dfb644@gmail.com>
In-Reply-To: <130c3636-b978-1600-df53-6a38c3414a88@gmx.de>

Hi Roland

On 01/07/2022 18:03, Roland Illig wrote:

Show 14 quoted lines

> Hi,
> 
> builtin/am.c says:
>  > static int str_isspace(const char *str)
>  > {
>  >     for (; *str; str++)
>  >         if (!isspace(*str))
>  >             return 0;
>  >
>  >     return 1;
>  > }
> 
> The macro 'isspace' must only be called with an integer representable as
> an 'unsigned char', or with the value of the macro EOF.

Thanks for reporting this. Git uses its own version of isspace() (see git-compat-util.h & ctype.c) which does not suffer from this limitation as it casts its argument to an unsigned char.

Best Wishes

Phillip

Show 12 quoted lines

> 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.
> 
> Roland