Re: [PATCH v2 2/8] packfile: use `repository` from `packed_git` directly

From: karthik nayak <karthik.188@gmail.com>
Date: Oct 29, 2024, 11:48 UTC
Message-ID: <CAOLa=ZS0A6sWciy3r8BbZcgVyzU2cdaG7S7JX4XwOQZbWcGwmQ@mail.gmail.com>
In-Reply-To: <Zx+3FZNzwe6V159m@nand.local>

Taylor Blau <me@ttaylorr.com> writes:

Show 37 quoted lines

> On Mon, Oct 28, 2024 at 02:43:40PM +0100, Karthik Nayak wrote:
>> In the previous commit, we introduced the `repository` structure inside
>> `packed_git`. This provides an alternative route instead of using the
>> global `the_repository` variable. Let's modify `packfile.c` now to use
>> this field wherever possible instead of relying on the global state.
>> There are still a few instances of `the_repository` usage in the file,
>> where there is no struct `packed_git` locally available, which will be
>> fixed in the following commits.
>>
>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>>  packfile.c | 50 +++++++++++++++++++++++++++-----------------------
>>  1 file changed, 27 insertions(+), 23 deletions(-)
>
> Very nice, and indeed much less disruptive than the RFC version of these
> patches. All of the first few transformations look correct to me.
>
>> @@ -751,9 +752,13 @@ struct packed_git *add_packed_git(struct repository *repo, const char *path,
>>  	p->pack_size = st.st_size;
>>  	p->pack_local = local;
>>  	p->mtime = st.st_mtime;
>> -	if (path_len < the_hash_algo->hexsz ||
>> -	    get_hash_hex(path + path_len - the_hash_algo->hexsz, p->hash))
>> -		hashclr(p->hash, the_repository->hash_algo);
>> +	if (path_len < repo->hash_algo->hexsz ||
>> +	    get_oid_hex_algop(path + path_len - repo->hash_algo->hexsz, &oid,
>> +			      repo->hash_algo))
>> +		hashclr(p->hash, repo->hash_algo);
>> +	else
>> +		memcpy(p->hash, oid.hash, repo->hash_algo->rawsz);
>
> This should be:
>
>     hashcpy(p->hash, oid.hash, repo->hash_algo);
>
> instead of a bare memcpy().
>

Indeed, didn't know of the function, will change.

> Everything else is looking good.
>
> Thanks,
> Taylor