Re: [PATCH] t4210: detect REG_ILLSEQ dynamically

From: Carlo Marcelo Arenas Belón <carenas@gmail.com>
Date: May 13, 2020, 21:04 UTC
Message-ID: <20200513210433.GA38566@Carlos-MBP>
In-Reply-To: <xmqqsgg3sh9v.fsf@gitster.c.googlers.com>

On Wed, May 13, 2020 at 01:37:32PM -0700, Junio C Hamano wrote:

Show 10 quoted lines

> Carlo Marcelo Arenas Belón <carenas@gmail.com> writes:
> 
> > indeed, I would rather go away with the whole prereq and set a variable
> > with a nice sounding name and use it below to `if test` the right tests,
> > would that be ok?
> 
> In a sense, a prerequisite is an overkill if the tests that need to
> be guarded are very well isolated and in a single place.  Do we
> expect other tests outside the context of this script may have to be
> guarded by REG_ILLSEQ?

IMHO in the long term we probably should get rid of those tests (including the need to look for REG_ILLSEQ), because the root cause of this failure is that the test is really relying in undefined behaviour which happen to be common to both glibc and our own compat regex, but that was relevant at that time, because of a bug on the way UTF-8 was being detected.

POSIX regcomp[1], specifically mentions that returned and error is a valid response to a badly encoded pattern, but the test cases were created to specifically ensure no error is ever generated, and to make sure the response (which is undefined, as per POSIX) was consistent for all engines:

"The regcomp() and regexec() functions are required to accept any null-terminated string as the pattern argument. If the meaning of the string is "undefined", the behavior of the function is "unspecified". IEEE Std 1003.1-2001 does not specify how the functions will interpret the pattern; they might return error codes, or they might do pattern matching in some completely unexpected way, but they should not do something like abort the process."

Carlo

[1] https://pubs.opengroup.org/onlinepubs/009695399/functions/regcomp.html