Re: [PATCH 6/8] midx: pass down `hash_algo` to `get_midx_filename[_ext]`

From: karthik nayak <karthik.188@gmail.com>
Date: Nov 18, 2024, 16:25 UTC
Message-ID: <CAOLa=ZTU08VGeATHDwnREiYT=PMevrTvG3BzffLCc8DBcHy-tQ@mail.gmail.com>
In-Reply-To: <ZzibPPpDw518npNp@ArchLinux>

shejialuo <shejialuo@gmail.com> writes:

Show 32 quoted lines

> On Fri, Nov 15, 2024 at 02:42:19PM +0100, Karthik Nayak wrote:
>
> [snip]
>
>> diff --git a/midx.h b/midx.h
>> index 78efa28d35371795fa33c68660278182debb60ab..7620820d4d0272926af9e4eeb68bfb73404c7ec2 100644
>> --- a/midx.h
>> +++ b/midx.h
>> @@ -7,6 +7,7 @@ struct object_id;
>>  struct pack_entry;
>>  struct repository;
>>  struct bitmapped_pack;
>> +struct git_hash_algo;
>>
>>  #define MIDX_SIGNATURE 0x4d494458 /* "MIDX" */
>>  #define MIDX_VERSION 1
>> @@ -89,8 +90,10 @@ struct multi_pack_index {
>>  #define MIDX_EXT_MIDX "midx"
>>
>>  const unsigned char *get_midx_checksum(struct multi_pack_index *m);
>> -void get_midx_filename(struct strbuf *out, const char *object_dir);
>> -void get_midx_filename_ext(struct strbuf *out, const char *object_dir,
>> +void get_midx_filename(const struct git_hash_algo *hash_algo,
>> +		       struct strbuf *out, const char *object_dir);
>> +void get_midx_filename_ext(const struct git_hash_algo *hash_algo,
>> +			   struct strbuf *out, const char *object_dir,
>>  			   const unsigned char *hash, const char *ext);
>
> I don't think it's a good idea to put "hash_algo" in the first argument,
> we should put it at the last to align with the code style where we use
> "git_hash_algo".
>

Could you elaborate on why you think it is not a good idea?

I've mostly done this to stay consistent, because I see `struct repository *repo` being passed as the first variable in our code base.

Roughly:

    $ grep -Iir "struct repository \*r" --include=\*.h | wc -l
    524

    $ grep -Iir "(struct repository \*r" --include=\*.h | wc -l
    327

Since `hash_algo` is similar, I thought it would be nicer to be consistent.

>>  void get_midx_chain_dirname(struct strbuf *buf, const char *object_dir);
>>  void get_midx_chain_filename(struct strbuf *buf, const char *object_dir);