On Wed, Sep 27, 2023 at 02:05:14PM -0600, Todd C. Miller wrote: > On Wed, 27 Sep 2023 10:59:26 -0600, "Todd C. Miller" wrote: > > > I think we want support for arbitrary line lengths. There is only > > one place where we need to reallocate the line buffer. > > The correct check is for "lp - line == linesz - 1". The code will > overwrite the newline with a NUL so we don't need to leave space > for it explicitly.
Yes, with your changes to the code the correct check is indeed lp - line == linesz - 1, as you always expand the buffer before writing anything to it, (even if it's just the null terminator). With my original patch it was necessary to use - 2 because we were inserting a new line character where there wasn't one before and with -1 it was writing the null to (line + LINE_MAX), which is one byte past the end of line[] if the input line was >= LINE_MAX. > As written, deroff will not emit a line that does not end with a > newline. That could be changed in a subsequent commit if it so > desired. That's always been the behaviour in every version that I've seen. It does have the possibly unexpected effect that /usr/bin/spell silently ignores the last line of input if it's not newline terminated. Whichever way we decide on for this, the future regression test should probably explicitly check for it, because parsing or not parsing an input line which is not newline terminated will obviously potentially affect scripts in non-obvious ways. But the patch looks OK to me and I much prefer handling arbitrary line lengths to the alternative of splitting them.
