Subject: Re: bug#66363: gdb-control-commands-regexp issues

From: Eli Zaretskii <[email-masked]>
Date: Thu, 05 Oct 2023 21:52:21 +0300

> From: Mattias Engdegård <[email-masked]>
> Date: Thu, 5 Oct 2023 20:11:24 +0200
> Cc: [email-masked]
> 
> 5 okt. 2023 kl. 19.43 skrev Eli Zaretskii <[email-masked]>:
> 
> > So the problem is only that 3 should be changed to the correct group
> > number?
> 
> That would perhaps work, but it wouldn't be a very robust solution. There are many groups preceding that we don't care about, and the bug arose precisely because they were added without considering that a group was being used.
> 
> Better then to get rid of all groups save the one in use. (The proposed patch does that.)
> That would make it much more difficult for the same bug to arise again.
> 
> > Here you are talking about some optimization of the regexp, or is it
> > another bug?  If the latter, what is the bug here?
> 
> It's related. When the reference to subgroup 3 was added (30c0f81f9f), the tail looked like this:
> 
>   "\\([[:blank:]]+\\([^[:blank:]]*\\)\\)?$"
>                   ^^^^^^^^^^^^^^^^^^^
> and subgroup 3 was the second group in this substring (underlined above). Later (f71afd600a) the last `?` was changed into a `*`, but that made the contents of that group somewhat hazy because of the repetition:
> 
>   "\\([[:blank:]]+\\([^[:blank:]]*\\)\\)*$"
>                                         ^
> which doesn't leave us with a useful group for the purpose of detecting command arguments at all.
> So the tail of the regexp has to be rewritten anyway, and we might as well do it in a more straightforward way. (The proposed patch does that as well.)
> 
> The reason I found this is that it contains a sub-pattern on the form (A+B*)* which is super-linear in general but usually easy to fix so that the result is actually more readable, yet faster.

Thanks.  I'm okay with the changes in principle, but someone will have
to test them by running all of the control commands and verifying they
work after the fix, before this can be installed.  I myself won't have
the time for doing that any time soon, sorry.