ping any feedback on code/approach highly appreciated Carlo On Wed, Aug 28, 2019 at 9:57 AM Carlo Arenas <carenas@xxxxxxxxx> wrote: > > FWIW, the changes in grep.h are IMHO optional and hadn't been really > tested as I couldn't find a version of PCRE1 old enough to trigger it > but I am hoping are simple enough to work. > > As in my original proposal, there is no test because this is going to > (as documented) trigger undefined behaviour and so I don't see how we > can reliably test that. Goes without saying tthough, that the same is > triggered when JIT is enabled and the JIT fast path is being used, so > this is not a regression and will be more visible once > ab/pcre-jit-fixes gets merged because we are moving out of the JIT > fast path with a patch[0] in that series > > While Ævar made a point[1] that this wasn't a solution to the problem, > it was because PCRE2 could have a better one (still missing but based > on ab/pcre-jit-fixes), and it was expected that PCRE2 will be used a > lot more and so it wasn't a good idea for it (something that doesn't > apply to PCRE1) > > the patch was based on maint (like all bugfixes) but applies cleanly > to master and next, it will conflict with pu but for easy of merge I'd > applied it on top of cb/pcre1-cleanup and make it available in > GitHub[2]; that branch could be used as well as a reroll for that > topic (if that is preferred) > > the error message hasn't been changed on this patch, as it might make > sense to improve it as well for PCRE2, but at least shouldn't be > triggered anymore (ex, from running a freshly built git without the > patch and linked against a non JIT enabled PCRE1): > > $ ./git-grep -P 'Nguyễn Thái.Ngọc' > .mailmap:Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> > fatal: pcre_exec failed with error code -10 > > Carlo > > [0] 685668faaa (grep: stop using a custom JIT stack with PCRE v1, 2019-07-26) > [1] https://public-inbox.org/git/87lfwn70nb.fsf@xxxxxxxxxxxxxxxxxxx/ > [2] https://github.com/carenas/git/tree/pcre1-cleanup