Re: [PATCH 0/2] pack-objects: missing tests & --stdin-packs segfault fix
From: Taylor Blau <hidden>
Date: 2021-06-21 20:34:38
On Mon, Jun 21, 2021 at 05:03:36PM +0200, Ævar Arnfjörð Bjarmason wrote:
When re-rolling an unrelated series[1] dealing with pack-objects.c and revision.c I discovered that we have some test blindspots, and that the newly added --stdin-packs option in v2.32.0 will segfault if fed garbage data. This fixes the test blindspots, and 2/2 fixes the segfault.
Thanks. I took a close look at the second patch, and it looks good to me with a few minor comments. The first patch looks good as well, although I didn't look as closely.
As discussed in its commit message I'm being lazy about emitting the error message. If you supply N bogus lines on stdin we'll error on the first one, since the input is first sorted by the string-list.c API. The test case for the error message relies on which of two SHA lines sorts first, and I picked input that happens to sort the same way under both SHA-1 and SHA-256. Lazy, but I figured for this use-case it wasn't worth keeping track of what line we saw when, or to refactor the parsing check on pack names as we get input lines.
Yeah. I think what you wrote is entirely reasonable, too. I suggested some alternatives if you are feeling motivated to make the error reporting nicer, but as you say, I think the vast majority of use-cases don't care about the output. Thanks, Taylor