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

From: Derrick Stolee <derrickstolee@github.com>
Date: Mar 2, 2022, 20:22 UTC
Message-ID: <66eeada0-f2b4-6849-bf14-029bb6c6083d@github.com>
In-Reply-To: <101b34660c0c5028ba591d052dc587bb8918ccb2.1646182671.git.me@ttaylorr.com>

On 3/1/2022 7:58 PM, Taylor Blau wrote:

Show 11 quoted lines

> To store the individual mtimes of objects in a cruft pack, introduce a
> new `.mtimes` format that can optionally accompany a single pack in the
> repository.
> 
> The format is defined in Documentation/technical/pack-format.txt, and
> stores a 4-byte network order timestamp for each object in name (index)
> order.
> 
> This patch prepares for cruft packs by defining the `.mtimes` format,
> and introducing a basic API that callers can use to read out individual
> mtimes.

...

Show 21 quoted lines

> +int load_pack_mtimes(struct packed_git *p)
> +{
> +	char *mtimes_name = NULL;
> +	int ret = 0;
> +
> +	if (!p->is_cruft)
> +		return ret; /* not a cruft pack */
> +	if (p->mtimes_map)
> +		return ret; /* already loaded */
> +
> +	ret = open_pack_index(p);
> +	if (ret < 0)
> +		goto cleanup;
> +
> +	mtimes_name = pack_mtimes_filename(p);
> +	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.

> +cleanup:
> +	free(mtimes_name);
> +	return ret;
> +}

Thanks, -Stolee