From: karthik nayak Date: Mon, 16 Dec 2024 15:59:57 GMT Subject: Re: [PATCH v3 8/8] refs: add support for migrating reflogs Message-ID: In-Reply-To: Junio C Hamano writes: > Patrick Steinhardt writes: > >>> +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? > We did [1] indeed, context is that I set it to '1' to distinguish between indexed and non-indexed updates. But it wasn't logically needed and was confusing so I decided to remove that change (which caused the issue here!). [1]: https://lore.kernel.org/r/CAOLa=ZQ9SHD3gzTVaznGhkCBjrrJbHm1fDyi1F-h6VZvtdpxgw@mail.gmail.com > Thanks.