PING. I have executed the ed tests (available in bin/ed/test) with this patch applied and the proposed patch doesn't seem to introduce any new regressions.
If the patch needs to be revised further, please let me know. The only thing I can think of right now is that it might make sense to rename the `recur` variable to `sawseparator` or something along those lines. Greetings, Sören Sören Tempel <soe...@soeren-tempel.net> wrote: > Hello, > > The POSIX specification of ed(1) includes a table in the rationale > section which (among others) mandates the following address handling > rules [1]: > > Address Addr1 Addr2 > ,, $ $ > ;; $ $ > > Unfortunately, OpenBSD does not correctly handle these two example > addresses. As an example, consider the following `,,p` print command: > > printf "a\nfoo\nbar\nbaz\n.\n,,p\nQ\n" | ed > > This should only print the last line (as `,,p` should expand to `$,$p`) > but instead prints all lines with OpenBSD ed. This seems to be a > regression introduced by Jerome Frgagic in 2017 [2]. Prior to this > change, `,,p` as well as `;;p` correctly printed the last line. The > aforementioned change added a recursive invocation of next_addr() which > does not update the local first variable (causing the second address > separator to be mistaken for the first). The patch below fixes this by > tracking recursive invocations of next_addr() via an additional function > parameter. > > See also: https://austingroupbugs.net/view.php?id=1582 > > [1]: > https://pubs.opengroup.org/onlinepubs/9699919799/utilities/ed.html#tag_20_38_18 > [2]: > https://github.com/openbsd/src/commit/b4c905bbf3932a50011ce3436b1e22acc742cfeb > > diff --git bin/ed/main.c bin/ed/main.c > index 231d021ef19..ca1aeb102b3 100644 > --- bin/ed/main.c > +++ bin/ed/main.c > @@ -64,7 +64,7 @@ void signal_hup(int); > void signal_int(int); > void handle_winch(int); > > -static int next_addr(void); > +static int next_addr(int); > static int check_addr_range(int, int); > static int get_matching_node_addr(regex_t *, int); > static char *get_filename(int); > @@ -296,7 +296,7 @@ extract_addr_range(void) > > addr_cnt = 0; > first_addr = second_addr = current_addr; > - while ((addr = next_addr()) >= 0) { > + while ((addr = next_addr(0)) >= 0) { > addr_cnt++; > first_addr = second_addr; > second_addr = addr; > @@ -328,7 +328,7 @@ extract_addr_range(void) > > /* next_addr: return the next line address in the command buffer */ > static int > -next_addr(void) > +next_addr(int recur) > { > char *hd; > int addr = current_addr; > @@ -382,11 +382,15 @@ next_addr(void) > case '%': > case ',': > case ';': > - if (first) { > + if (first && !recur) { > ibufp++; > addr_cnt++; > second_addr = (c == ';') ? current_addr : 1; > - if ((addr = next_addr()) < 0) > + > + // If the next address is omitted (e.g. "," or > ";") or > + // if it is also a separator (e.g. ",," or > ";;") then > + // return the last address (as per the omission > rules). > + if ((addr = next_addr(1)) < 0) > addr = addr_last; > break; > }
