Re: [PATCH v2 1/1] midx.c: fix an integer overflow

From: Damien Robert <damien.olivier.robert@gmail.com>
To: Derrick Stolee <stolee@gmail.com>
Cc: Jeff King <peff@peff.net>,
	git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	Derrick Stolee <dstolee@microsoft.com>,
	William Baker <William.Baker@microsoft.com>
Subject: Re: [PATCH v2 1/1] midx.c: fix an integer overflow
Date: Thu, 12 Mar 2020 22:41:23 +0100	[thread overview]
Message-ID: <20200312214123.sqja3fvuiaaspxwp@doriath> (raw)
In-Reply-To: <a538c497-a79d-43be-3d00-c7a619acc4e6@gmail.com>

From Derrick Stolee, Thu 12 Mar 2020 at 14:28:11 (-0400) :
> I _think_ the t/tXXXX directories are used for this kind of data storage,
> so you could generate an empty multi-pack-index from an older version of
> Git then store it there.

Yes I anticipated that and have one available on hand :)
It weights 1116 characters.

> > -	if (flags & MIDX_PROGRESS)
> > -		progress = start_sparse_progress(_("Verifying OID order in multi-pack-index"),
> > -						 m->num_objects - 1);
> > -	for (i = 0; i < m->num_objects - 1; i++) {
> > -		struct object_id oid1, oid2;
> > +	if (m->num_objects == 0)
> > +		midx_report(_("Warning: the midx contains no oid."));
> 
> Should this "Warning: " be here? The other calls to midx_report() do not have such prefix.

Right, I agree it should not.

> Also, it may be valuable to return from this block so you do not need to put the block below in a tabbed block, reducing the complexity of this patch.

Agreed: we don't want to run the other checks anyway if we don't have any
objects.

That'll be for v3 once I get advice on what to do for tests.