Re: [PATCH v3 8/8] refs: add support for migrating reflogs

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 16, 2024, 15:50 UTC
Message-ID: <xmqqo71b7hkn.fsf@gitster.g>
In-Reply-To: <Z1_KzlKc7RBfas4L@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 22 quoted lines

>> +static int migrate_one_reflog(const char *refname, void *cb_data)
>> +{
>> +	struct migration_data *migration_data = cb_data;
>> +	struct reflog_migration_data data;
>> +
>> +	data.refname = refname;
>> +	data.old_refs = migration_data->old_refs;
>> +	data.transaction = migration_data->transaction;
>> +	data.errbuf = migration_data->errbuf;
>> +	data.sb = &migration_data->sb;
>
> The `index` variable isn't getting initialized here anymore, so its
> value is essenitally random. I'd propose to use designated initializers
> for `data` to fix this:
>
>     struct reflog_migration_data data = {
>         .refname = refname,
>         .old_refs = migration_data->old_refs,
>         .transaction = migration_data->transaction,
>         .errbuf = migration_data->errbuf,
>         .sb = &migration_data->sb,
>     };

GOod. As long as it is sensible to null-initialize the relevant field and all the other fields not mentioned above, that certainly would give us more predicitable behaviour ;-). I do not offhand know if 0 is the right value to initialize the .index member with, though; didn't you two recently had an exchange about starting with 0 or 1 or something?

Thanks.