On Tue, Jul 21, 2020 at 11:55:39AM +0200, Hiltjo Posthuma wrote:
> 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).
>
Hmm. I get neither a seg fault nor a loop on my amd64 Lenove E595. The
cursor moves to different spots in X/i3 xterm vs console and in either
case doesn't do any replacement.
.... Ken
> > > 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(®ex_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
>
