On Mon, Jul 20, 2020 at 10:11:46PM +0200, Theo Buehler wrote:
> On Mon, Jul 13, 2020 at 03:47:13PM +0100, Mark Willson wrote:
> > Folks,
> > 
> > The segmentation fault occurs when the string to replace is just the
> > "^" anchor (beginning-of-line) and the point is on an empty line.
> > Issue occurs on a -current system dated July 11th. (dmesg below).
> > 

I can reproduce the issue, but I don't get a segmentation fault but an infinite
loop here on -current and on 6.6 also (it is not a regression from the previous
patch).

> > The following patch prevents the segmentation fault:
> 
> Nice. Thanks for the patches and the analysis. I'd suggest the following
> variant (only stylistic changes).  I'm willing to commit this, but I
> will need at least some positive feedback from mg users.
> 
> Index: line.c
> ===================================================================
> RCS file: /var/cvs/src/usr.bin/mg/line.c,v
> retrieving revision 1.61
> diff -u -p -r1.61 line.c
> --- line.c    29 Aug 2018 07:50:16 -0000      1.61
> +++ line.c    13 Jul 2020 16:00:52 -0000
> @@ -556,6 +556,8 @@ lreplace(RSIZE plen, char *st)
>               goto done;
>  
>       lp = curwp->w_dotp;
> +     if (ltext(lp) == NULL)
> +             goto done;
>       doto = curwp->w_doto;
>       n = plen;
>  
> Index: re_search.c
> ===================================================================
> RCS file: /var/cvs/src/usr.bin/mg/re_search.c,v
> retrieving revision 1.34
> diff -u -p -r1.34 re_search.c
> --- re_search.c       9 Jul 2020 10:42:24 -0000       1.34
> +++ re_search.c       13 Jul 2020 16:17:07 -0000
> @@ -308,7 +308,7 @@ re_doreplace(RSIZE plen, char *st)
>  static int
>  re_forwsrch(void)
>  {
> -     int      tbo, tdotline, error;
> +     int              re_flags, tbo, tdotline, error;
>       struct line     *clp;
>  
>       clp = curwp->w_dotp;
> @@ -318,9 +318,10 @@ re_forwsrch(void)
>       if (tbo == clp->l_used)
>               /*
>                * Don't start matching past end of line -- must move to
> -              * beginning of next line, unless at end of file.
> +              * beginning of next line, unless line is empty or at
> +              * end of file.
>                */

I think the below line should stay the same, else forward searching on
empty lines does not work:

> -             if (clp != curbp->b_headp) {
> +             if (clp != curbp->b_headp && llength(clp) != 0) {


>                       clp = lforw(clp);
>                       tdotline++;
>                       tbo = 0;
> @@ -330,10 +331,13 @@ re_forwsrch(void)
>        * always makes the last line empty so this is good.
>        */
>       while (clp != (curbp->b_headp)) {
> +             re_flags = REG_STARTEND;
> +             if (tbo != 0)
> +                     re_flags |= REG_NOTBOL;
>               regex_match[0].rm_so = tbo;
>               regex_match[0].rm_eo = llength(clp);
>               error = regexec(&regex_buff, ltext(clp) ? ltext(clp) : "",
> -                 RE_NMATCH, regex_match, REG_STARTEND);
> +                 RE_NMATCH, regex_match, re_flags);
>               if (error != 0) {
>                       clp = lforw(clp);
>                       tdotline++;
> 

Tested on -current and with the above change looks good to me, thanks!

-- 
Kind regards,
Hiltjo

Reply via email to