Re: [PATCH 08/20] midx: enforce chunk alignment on reading
- From
Jeff King <peff@peff.net>
- Date
- Oct 11, 2023, 23:09 UTC
- Message-ID
- <20231011230927.GJ518221@coredump.intra.peff.net>
- In-Reply-To
- <ZSa4tfG2NyM1s9Jz@nand.local>
On Wed, Oct 11, 2023 at 11:01:09AM -0400, Taylor Blau wrote:
Show 17 quoted lines
> On Mon, Oct 09, 2023 at 05:05:23PM -0400, Jeff King wrote:
> > @@ -120,6 +121,11 @@ int read_table_of_contents(struct chunkfile *cf,
> > error(_("terminating chunk id appears earlier than expected"));
> > return 1;
> > }
> > + if (chunk_offset % expected_alignment != 0) {
>
> Oops, I missed this in my first read. I'm definitely nit-picking here,
> but this should probably be:
>
> if (chunk_offset % expected_alignment)
>
> without the trailing "!= 0".
>
> I don't have a strong preference here, since we are doing a comparison
> of an arithmetic operation against an (un-)expected value. But I think
> the CodingGuidelines would technically call this out of style...I suppose so, but somehow I consider the subtlety of "%" to merit the more explicit comparison (versus something like "if (foo)"). Grepping for "if (.* %)' seems to show some of both.
-Peff