On Mon, Oct 09, 2023 at 04:55:44PM -0400, Jeff King wrote: > bloom.c | 34 +++++++++ > chunk-format.c | 24 ++++-- > chunk-format.h | 9 ++- > commit-graph.c | 119 ++++++++++++++++++++++++----- > commit-graph.h | 4 + > midx.c | 68 +++++++++++++---- > midx.h | 3 + > pack-revindex.c | 13 +++- > t/lib-chunk.sh | 17 +++++ > t/lib-chunk/corrupt-chunk-file.pl | 66 ++++++++++++++++ > t/t4216-log-bloom.sh | 50 ++++++++++++ > t/t5318-commit-graph.sh | 76 +++++++++++++++++- > t/t5319-multi-pack-index.sh | 102 ++++++++++++++++++++++++- > t/t5324-split-commit-graph.sh | 20 ++++- > t/t5328-commit-graph-64bit-time.sh | 10 +++ > 15 files changed, 568 insertions(+), 47 deletions(-) > create mode 100644 t/lib-chunk.sh > create mode 100644 t/lib-chunk/corrupt-chunk-file.pl I reviewed this carefully (well, except for the new Perl script, for obvious[^1] reasons ;-)). Everything mostly looks good to me, though I had a handful of review comments throughout. Many of them are trivial (e.g. a number of warning() and error() strings should be marked for translation, etc.), but a couple of them I think are worth looking at. Most notably, I think that by the end of the series, I was convinced that having some kind of 'pair_chunk_expectsz()' or similar would be useful and eliminate a good chunk of the boilerplate you have to write to check the chunk size against an expected value when using read_chunk(). Otherwise, this looks great. I appreciate the care you took in finding and fixing these issues, as well as thoroughly documenting the process (and the security implications, or lack thereof). Thanks for working on this! Thanks, Taylor [^1]: That I may be the world's least competent Perl programmer.
