Update of bug #68064 (group groff):
Status: None => Fixed
Assigned to: None => gbranden
Open/Closed: Open => Closed
Planned Release: None => 1.24.0
Summary: [troff] `sy` request does not populate `systat`
register => [troff] `sy` request only belatedly populates `systat` register
(mishandles input stream pointer)
_______________________________________________________
Follow-up Comment #18:
[comment #17 comment #17:]
> [comment #16 comment #16:]
>> Of course the opposite may have been happening:
>
> Nope, it looks like in 1.23, .tm was somehow "cementing" the systat value.
> $ cat bad-sy-3.groff
> .sy exit 2
> .tm stderr: \n[systat]
> \n[systat]
> $ fgrep -v .tm bad-sy-3.groff | groff -aU
> <beginning of page>
> 0
> $ groff -aU bad-sy-3.groff
> stderr: 512
> <beginning of page>
> 512
> $
And all of a sudden my spidey-sense tingled.
Know what else "cements" it?
$ cat ATTIC/bad-sy-3gbr.groff
.sy exit 2
.
\n[systat]
$ grep -Fvx . ATTIC/bad-sy-3gbr.groff | ~/groff-1.23.0/bin/groff -aU
<beginning of page>
0
$ ~/groff-1.23.0/bin/groff -aU ATTIC/bad-sy-3gbr.groff
<beginning of page>
512
And with that I think we can almost solve the mystery. Recall comment #5.
Observe that _groff_ 1.23.0's `system_request()` does not call `token.next()`
(if it does any work at all).
_groff_ 1.24.0's **does**.
That was a bug fix.
Deri said in bug #66512 the problem came in with my commit commit 24e314b9,
2024-11-12. This report and our findings suggest that it's actually an older
problem. It could be that my changes in the commit Deri spotted made the
problem occur in more scenarios.
If we look at that change with a lot of diff context to sweep up the whole
(then-named) `read_string()` function...
// Consume the rest of the input line in copy mode; if, after spaces,
// the argument starts with a `"`, discard it, letting any immediately
// subsequent spaces populate the returned string.
+//
+// The caller must use `tok.next()` to advance the input stream pointer.
char *read_string()
{
int len = 256;
char *s = new char[len]; // C++03: new char[len]();
(void) memset(s, 0, (len * sizeof(char)));
int c = get_copy(0 /* nullptr */);
while (c == ' ')
c = get_copy(0 /* nullptr */);
if (c == '"')
c = get_copy(0 /* nullptr */);
int i = 0;
while (c != '\n' && c != EOF) {
if (!is_invalid_input_char(c)) {
if (i + 2 > len) {
char *tem = s;
s = new char[len * 2]; // C++03: new char[len * 2]();
(void) memset(s, 0, (len * 2 * sizeof(char)));
memcpy(s, tem, len);
len *= 2;
delete[] tem;
}
s[i++] = c;
}
c = get_copy(0 /* nullptr */);
}
s[i] = '\0';
- tok.next();
if (i == 0) {
delete[] s;
return 0 /* nullptr */;
}
return s;
}
...then it looks to me like this function never actually advanced the input
stream pointer (called `tok.next()`) after reading the _final_ character in a
rest-of-line argument. But it also didn't stuff that final character into the
C string the function returned, so there wasn't stray garbage in that string
that made the misbehavior easier to detect.
So why didn't the `systat` register get populated?
Answer: It does. But later.
What appears to happen is that the next line of input after a `sy` request
_gets ignored_. The "argument" to the `sy` request has already been gathered
and processed, and the formatter enters some kind of no-man's-land in terms of
parser state where it's not handling a request but also not reading a text
line, because it hasn't reached the beginning of a new input line yet to make
the decision of what sort of line it's reading. It thinks it's still
processing a control line, but no request handler function is active to
consume input.
Evidence?
Well, I checked out _groff_ 1.23.0, added a bit of instrumentation to
`system_request()` and enhanced our minimal test inputs to be SLIGHTLY less
minimal. Observe.
(This is interactive input, so you see both input and output interleaved. I
hope it's still intelligible.)
$ ./build/test-groff -aU
.sy exit 2
.
troff:<standard input>:2: error: GBR: DING! `system_status` set to 512
$ ./build/test-groff -aU
.sy exit 2
\n[systat]
troff:<standard input>:2: error: GBR: DING! `system_status` set to 512
<beginning of page>
\n[systat]
0 512
That means that the latter example should behave the same with stock _groff_
1.23.0, just without the bell going off.
$ cat ATTIC/bad-sy-4.groff
.sy exit 2
\n[systat]
\n[systat]
$ ~/groff-1.23.0/bin/groff -aU ATTIC/bad-sy-4.groff
<beginning of page>
0 512
...and sure enough it does.
The net result is that we're able to mount another head on _groff_ 1.24.0's
wall.
It took three tries to fix, though.
commit 5f2704d64a, 2024-09-02
commit 24e314b94c, 2024-11-12
commit 7ba0196fb4, 2024-12-02
My assumption that `sy` was working correctly in the first place may explain
some of my difficulty.
_______________________________________________________
Reply to this item at:
<https://savannah.gnu.org/bugs/?68064>
_______________________________________________
Message sent via Savannah
https://savannah.gnu.org/
signature.asc
Description: PGP signature
