On Tue, 27 Jun 2017 09:29:10 +0000, Otto Moerbeek wrote: > > at last a followup, for the original problem. > > This diff incorporates your later comment. It does not cause the newly > added regress test to fail, though. > > So that poses the question if this is what you meant. > > -Otto > > Index: process.c > =================================================================== > RCS file: /cvs/src/usr.bin/sed/process.c,v > retrieving revision 1.32 > diff -u -p -r1.32 process.c > --- process.c 22 Feb 2017 14:09:09 -0000 1.32 > +++ process.c 27 Jun 2017 09:16:33 -0000 > @@ -120,8 +120,10 @@ redirect: > cp = cp->u.c; > goto redirect; > case 'c': > + if (pd) > + break; > pd = 1; > - psl = 0; > + ps[psl = 0] = '\0'; > if (cp->a2 == NULL || lastaddr || lastline()) > (void)fprintf(outfile, "%s", cp->t); > break; > @@ -138,6 +140,7 @@ redirect: > } else { > psl -= (p + 1) - ps; > memmove(ps, p + 1, psl); > + ps[psl] = '\0'; > goto top; > } > case 'g': >
Indeed, there was a wording error in my second paragraph: I meant to align the behaviour of `l' (not `c') with that of `p', such that using `l' after `c' would not print anything (not even `$'), just like `p' in this case, which does not print anything either (not even a newline). Index: process.c =================================================================== RCS file: /cvs/src/usr.bin/sed/process.c,v retrieving revision 1.32 diff -up -r1.32 process.c --- process.c 22 Feb 2017 14:09:09 -0000 1.32 +++ process.c 1 Jul 2017 10:24:21 -0000 @@ -121,7 +121,7 @@ redirect: goto redirect; case 'c': pd = 1; - psl = 0; + ps[psl = 0] = '\0'; if (cp->a2 == NULL || lastaddr || lastline()) (void)fprintf(outfile, "%s", cp->t); break; @@ -138,6 +138,7 @@ redirect: } else { psl -= (p + 1) - ps; memmove(ps, p + 1, psl); + ps[psl] = '\0'; goto top; } case 'g': @@ -158,6 +159,8 @@ redirect: (void)fprintf(outfile, "%s", cp->t); break; case 'l': + if (pd) + break; lputs(ps); break; case 'n': That being said, there is more to this issue than what such a simple patch could fix. For example, in a way your diff also makes sense: if it is believed that one ought not to use `l' nor `p' to print an inexistant pattern space, then surely deleting it again with `c' should be disallowed too. However, as partly demonstrated by my last message, there is absolutely no coherence anywhere about this, so in the end one might wonder which command is correctly behaved and which isn't. For instance, it is a matter of course that the sequence `N;s/.*\n//;p' should be just the same as `n' when normal output has not been disabled; but, in OpenBSD's sed, because that `pd' flag is implemented in such an erratic fashion, this isn't always the case: $ jot 6 | sed 'c\ > text > :loop > N;s/.*\n//;p > bloop' text 2 4 6 $ jot 6 | sed 'c\ > text > :loop > n > bloop' text 2 3 4 5 6 For the same reason, nothing is printed by the following script, even if all lines are read: $ jot 6 | sed 'c\ > text > :loop > $q;N > bloop' text (As a side note, amusingly, replacing the `q' above with `s/$//p' would actually produce the expected output, when in contrast using something like `s/^//p' would not... But this is an unrelated bug.) Likewise, one would believe the command `y/a/b/' to be functionally equivalent to `s/a/b/g'; but, again, many examples of the opposite behaviour can be constructed; for instance: $ echo | sed 'c\ > text > s/^/a/;s/^// > y/a/b/ > s/^//p;d' text a $ echo | sed 'c\ > text > s/^/a/;s/^// > s/a/b/g > s/^//p;d' text b Because, of course, thanks to that `pd' magic, `y' is only effective after `c' when an odd number of substitutions have affected the pattern space. Although it is true that the above two bugs are primarily due to the incorrect handling of the `deleted' flag by the `s' command, they nonetheless help to expose further the inconsistency of the code, making its overall fragility even more obvious than it would have otherwise been; in this case in particular, they allow one to notice that: 1. Even though `n' resets the `deleted' flag, `N' does not. 2. Even though `s' ignores the `deleted' flag, `y' does not. And so, similarily, even if `p', `P', and `w' check for it, `l' ignores `pd' completely; and, indeed, so does `c', in contrast with other commands such as `D'. I hope that these examples make it clear that the problem is much worse than what the above diff actually solves. Of course, I could have written a third detailed report appropriately addressing all of this, but I figured no reasonable amount of patching endeavour could ever truly and satisfyingly improve the overall condition of such inherently flawed code. Therefore, I no longer believe any of my patches (nor any other patches, really) to be relevant to OpenBSD's current sed. This might sound a little extreme, but, in fact, as my previous messages illustrate, that `pd' madness is far from being the only incongruity lurking within sed's source. I should also add that, since then, I actually found more bugs, some of them even affecting the parsing algorithm... To take only one example out of many, the implementation of the `y' command is incorrect: $ sed 'y/[/]/' sed: 1: "y/[/]/": unterminated transform target string (The parser thinks that an opening bracket in an argument to the `y' command is meant to be the start of a character class, as if it were part of a regular expression.) Considering that sed is my favourite Unix utility (dc comes close; vi and sh don't count as they are more than mere utilities), I would be very sad if all my beautiful sed scripts were forever to be handled by such a mediocre implementation, since I don't really plan to ever use anything else than OpenBSD. Also, in light of all these observations, instead of devising endless diffs for a piece of software that clearly doesn't deserve any, I am willing to rewrite it from scratch. This might take a few weeks as I do not have enough time to work on it regularly, but there is apparently no hurry: after all, I seem to be the only one out there to actually need such features as empty match replacement to work reliably, no matter if the last command was `D' or if there happens to be an embed NUL or newline in the pattern space. Besides, and perhaps even more importantly, a clean rewrite could yield many other improvements (like a more restrictive pledge(2) depending on the outcome of the scripts' compilation), without the hassle of having to untangle such old and messy code. As a consequence, especially since no one at OpenBSD appears to have time to deal with all the deficiencies of the existing source, it might be simpler to wait until I am done with my own, as my code shall be much more readable than the current one, and therefore quite easy to review. Kind regards, kshe