Re: mg: crash on re-search with a file containing an empty line

[Hiltjo Posthuma]

On Mon, Jun 29, 2020 at 02:36:31PM +0200, Theo Buehler wrote:
> On Mon, Jun 29, 2020 at 01:17:56PM +0200, Hiltjo Posthuma wrote:
> > Hi,
> > 
> > Thanks for looking into it, it looks better.
> > 
> > With this patch I noticed re_backsrch() does not work fully correct yet.
> > 
> > I think the line:
> > 
> > > +             regex_match[0].rm_so < tbo) {
> > 
> > should be:
> > 
> > > +             regex_match[0].rm_so <= tbo) {
> > 
> > because tbo is already adjusted at the top of the function re_backsrch().
> > 
> > With this line change searching backwards with "^$" finds a result.
> 
> Indeed, thank you. I must have messed up my testing...
> 
> However, changing < to <= changes the behavior of all backward regex
> searches.
> 
> Type "aaa" on a line and leave the cursor after the third a on the same
> line. If you re-search-backward for "a" on -current, the cursor is
> placed on the middle "a" (different from emacs which places the cursor
> on the third a).  If you then re-search-backward for "aa", the search
> fails (as it does on emacs also).
> 
> With your suggested change (as in the diff below), the cursor is placed
> on the third "a" as in emacs. However, with the cursor on the middle a
> re-search-backward for "aa" now succeeds (different from emacs).
> 
> I'm not sure this is desirable. I'm no user of either emacs or mg, so I
> think more thought and care is needed but this will have to be done by
> someone else...
> 
> Index: re_search.c
> ===================================================================
> RCS file: /var/cvs/src/usr.bin/mg/re_search.c,v
> retrieving revision 1.33
> diff -u -p -r1.33 re_search.c
> --- re_search.c       6 Aug 2017 04:39:45 -0000       1.33
> +++ re_search.c       29 Jun 2020 11:26:07 -0000
> @@ -332,8 +332,8 @@ re_forwsrch(void)
> while (clp != (curbp->b_headp)) {
>               regex_match[0].rm_so = tbo;
>               regex_match[0].rm_eo = llength(clp);
> -             error = regexec(&regex_buff, ltext(clp), RE_NMATCH, regex_match,
> -                 REG_STARTEND);
> +             error = regexec(&regex_buff, ltext(clp) ? ltext(clp) : "",
> +                 RE_NMATCH, regex_match, REG_STARTEND);
>               if (error != 0) {
>                       clp = lforw(clp);
>                       tdotline++;
> @@ -389,8 +389,9 @@ re_backsrch(void)
>                * do this character-by-character after the first match since
>                * POSIX regexps don't give you a way to do reverse matches.
>                */
> -             while (!regexec(&regex_buff, ltext(clp), RE_NMATCH, regex_match,
> -                 REG_STARTEND) && regex_match[0].rm_so < tbo) {
> +             while (!regexec(&regex_buff, ltext(clp) ? ltext(clp) : "",
> +                 RE_NMATCH, regex_match, REG_STARTEND) &&
> +                 regex_match[0].rm_so <= tbo) {
>                       memcpy(&lastmatch, &regex_match[0], sizeof(regmatch_t));
>                       regex_match[0].rm_so++;
>                       regex_match[0].rm_eo = llength(clp);
> @@ -538,8 +539,8 @@ killmatches(int cond)
>               /* see if line matches */
>               regex_match[0].rm_so = 0;
>               regex_match[0].rm_eo = llength(clp);
> -             error = regexec(&regex_buff, ltext(clp), RE_NMATCH, regex_match,
> -                 REG_STARTEND);
> +             error = regexec(&regex_buff, ltext(clp) ? ltext(clp) : "",
> +                 RE_NMATCH, regex_match, REG_STARTEND);
>  
>               /* Delete line when appropriate */
>               if ((cond == FALSE && error) || (cond == TRUE && !error)) {
> @@ -613,8 +614,8 @@ countmatches(int cond)
>               /* see if line matches */
>               regex_match[0].rm_so = 0;
>               regex_match[0].rm_eo = llength(clp);
> -             error = regexec(&regex_buff, ltext(clp), RE_NMATCH, regex_match,
> -                 REG_STARTEND);
> +             error = regexec(&regex_buff, ltext(clp) ? ltext(clp) : "",
> +                 RE_NMATCH, regex_match, REG_STARTEND);
>  
>               /* Count line when appropriate */
>               if ((cond == FALSE && error) || (cond == TRUE && !error))
> 

Hi,

I don't use (GNU/)emacs either.

Can this patch get committed in the current form?

-- 
Kind regards,
Hiltjo