Hi, I've been using this patch for the past 6 months and haven't noticed any regressions. However, during this timespan I became aware of additional POSIX compatibility issues in OpenBSD ed: `3 ---- 2p` as well as addresses with interleaved `,` and `;` separators (e.g. `1,;`) are also evaluated incorrectly.
These additional compatibility issues are unrelated to the issue fixed by the proposed patch. Is there a general interest in improving the POSIX compatibility of OpenBSD ed? If so, what would need to be done in order to get the proposed patch committed? Greetings, Sören Sören Tempel <[email protected]> wrote: > 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 <[email protected]> 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; > > }
