Making an elephant out of a getline() bug
Get series with b4
b4 am [email protected]Versions newest first
- v2 Latest4 patches 3 repliesIn review
Patches in v2 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/4 | strbuf: improve strbuf_get*line documentationDocumentation/ | +10 | 1 reply | |
| 2/4 | bundle: put strbuf_readline_fd in strbuf.c with adjustmentsDocumentation/ bundle.c strbuf.c +1 more | +26 | 0 replies | |
| 3/4 | t5704: match tests to modern stylet/ | +8 | 1 reply | |
| 4/4 | bundle: use a strbuf to scan the log for boundary commitsbundle.c t/ | +24 | 0 replies |
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v2 yet.
Cover letter [PATCH v2 0/4] Making an elephant out of a getline() bug
So you all made very good points, and I don't want to repeat them. Junio's analysis
Quoted 6 lines
Perhaps we would want to squash something like this to the test to avoid "seq", using J6t's idea. The issue is that we do not write the end of line for the long boundary (because it is hidden from us), keep reading and rejecting bogus letters, and then the tip is written without terminating the boundary line. So checking boundary line is not a very useful test, but "fetch" and "list-heads" are.
is of course also correct for the case where the overlong line is in a boundary commit. (The analysis about the accidentally-leading '-' is also correct and can create a boundary line where there was none, like in the perl case.)
The patches in this round are:
[1] new; following up on Peff's complaint that I should document strbuf_getwholeline_fd, I found that the strbuf_get*line documentation was also lacking.
[2] previously (1/2), now with documentation added
[3] new; Junio corrected me on the ': > file' spelling, which I added because the start of the test used it too. So let's just fix it outright to match the Git style all over.
[4] previously (2/2), now incorporating a strbuf_release and the suggestions for the tests as sent by Junio. I upped the %0982 a bit as it felt *too* tailored for the old bug, and this way everyone can see immediately that it will exceed the 1024 char limit (without thinking about the length of a sha and accounting for \0 and \n).
Thomas Rast (4):
strbuf: improve strbuf_get*line documentation
bundle: put strbuf_readline_fd in strbuf.c with adjustments
t5704: match tests to modern style
bundle: use a strbuf to scan the log for boundary commits
Documentation/technical/api-strbuf.txt | 19 +++++++++++-- bundle.c | 36 +++++++----------------- strbuf.c | 16 +++++++++++ strbuf.h | 1 + t/t5704-bundle.sh | 47 ++++++++++++++++---------------- 5 files changed, 67 insertions(+), 52 deletions(-)
Signature · 1 line
1.7.9.1.430.g4998543