From: Phillip Wood Date: Wed, 26 Apr 2023 14:44:08 GMT Subject: Re: [PATCH v2 3/4] parse_commit(): handle broken whitespace-only timestamp Message-ID: <01709d21-d641-cfcd-c389-da9aaf84f5be@gmail.com> In-Reply-To: <87mt2u3eav.fsf@igel.home> Hi Andreas On 26/04/2023 15:31, Andreas Schwab wrote: > On Apr 26 2023, Phillip Wood wrote: > >> On 25/04/2023 17:06, Junio C Hamano wrote: >>> Phillip Wood writes: >>> >>>> This probably doesn't matter in practice but we define our own >>>> isspace() that does not treat '\v' and '\f' as whitespace. However >>>> parse_timestamp() (which is just strtoumax()) uses the standard >>>> library's isspace() which does treat those characters as whitespace >>>> and is locale dependent. This means we can potentially stop at a >>>> character that parse_timestamp() treats as whitespace and if there are >>>> no digits after it we'll still walk past the end of the line. Using >>>> Rene's suggestion of testing the character with isdigit() would fix >>>> that. It would also avoid parsing negative timestamps as positive >>>> numbers >> >>>> and reject any timestamps that begin with a locale dependent >>>> digit. >> >> Sorry, that bit is not correct, I've since checked the C standard and I >> think strtoul() and friends expect ascii digits (isdigit() and isxdigit() >> are also locale independent unlike isspace(), isalpha() etc.) > > The standard says: > > In other than the "C" locale, additional locale-specific subject > sequence forms may be accepted. Thanks, looking at the standard again I don't know how I managed to miss that, my initial recollection was correct after all. Best Wishes Phillip