git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Carlo Marcelo Arenas Belón" <carenas@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Eric Sunshine <sunshine@sunshineco.com>,
	Git List <git@vger.kernel.org>, Ed Maste <emaste@freebsd.org>
Subject: Re: [PATCH] t4210: detect REG_ILLSEQ dynamically
Date: Wed, 13 May 2020 14:04:33 -0700	[thread overview]
Message-ID: <20200513210433.GA38566@Carlos-MBP> (raw)
In-Reply-To: <xmqqsgg3sh9v.fsf@gitster.c.googlers.com>

On Wed, May 13, 2020 at 01:37:32PM -0700, Junio C Hamano wrote:
> 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

  reply	other threads:[~2020-05-13 21:04 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-13 11:16 [PATCH] t4210: detect REG_ILLSEQ dynamically Carlo Marcelo Arenas Belón
2020-05-13 15:44 ` Eric Sunshine
2020-05-13 16:20   ` Junio C Hamano
2020-05-13 20:18   ` Carlo Marcelo Arenas Belón
2020-05-13 20:37     ` Junio C Hamano
2020-05-13 21:04       ` Carlo Marcelo Arenas Belón [this message]
2020-05-13 18:02 ` [RFC PATCH v2] " Carlo Marcelo Arenas Belón
2020-05-13 20:40   ` Eric Sunshine
2020-05-15 18:00   ` Junio C Hamano
2020-05-15 18:18     ` Carlo Marcelo Arenas Belón
2020-05-15 19:51 ` [PATCH " Carlo Marcelo Arenas Belón
2020-05-15 20:24   ` Junio C Hamano
2020-05-15 21:48     ` Junio C Hamano
2020-05-18 18:44   ` [PATCH v3 0/2] auto detect REG_ILLSEQ Carlo Marcelo Arenas Belón
2020-05-18 18:44     ` [PATCH v3 1/2] t/helper: teach test-regex to report pattern errors (like REG_ILLSEQ) Carlo Marcelo Arenas Belón
2020-05-18 20:15       ` Junio C Hamano
2020-05-18 18:44     ` [PATCH v3 2/2] t4210: detect REG_ILLSEQ dynamically and skip affected tests Carlo Marcelo Arenas Belón

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200513210433.GA38566@Carlos-MBP \
    --to=carenas@gmail.com \
    --cc=emaste@freebsd.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).