Re: [PATCH v2 02/17] pack-mtimes: support reading .mtimes files

From: Taylor Blau <me@ttaylorr.com>
Date: Mar 2, 2022, 21:33 UTC
Message-ID: <Yh/isYFFmmOdpNa7@nand.local>
In-Reply-To: <66eeada0-f2b4-6849-bf14-029bb6c6083d@github.com>

On Wed, Mar 02, 2022 at 03:22:18PM -0500, Derrick Stolee wrote:

Show 11 quoted lines

> > +	ret = load_pack_mtimes_file(mtimes_name,
> > +				    p->num_objects,
> > +				    &p->mtimes_map,
> > +				    &p->mtimes_size);
> > +	if (ret)
> > +		goto cleanup;
>
> This looked odd to me, so I supposed that you had some code
> that would be inserted between this 'goto cleanup' and the
> 'cleanup:' label, but I did not find such an insertion in
> the remaining patchs. This 'if' can be deleted.

Thanks for spotting. My gut was that there must be something in the range-diff between this and the previous round, but there isn't. So this code has always been there.

It likely comes from load_pack_revindex_from_disk(), which assigns the `revindex_data` member of `struct packed_git` after calling load_revindex_from_disk(), but only if it returned zero.

We don't have to assign mtimes_data here (since it doesn't exist, and) because all of our reads into mtimes_map are offset by 3 to adjust for the width of the header.

Anyway, we don't need this if statement here, so I'll drop it.

Thanks, Taylor