On 1/12/2023 7:55 AM, Ævar Arnfjörð Bjarmason wrote: > * Derrick suggested in > https://lore.kernel.org/git/6b92fad2-6b74-fddb-679c-8c8735e7103d@xxxxxxxxxx/ > that things might be nicer if we had an explicit initializer, which > was also the subject of the previous discussion at > https://lore.kernel.org/git/xmqqmt6vqo2w.fsf@gitster.g/; but > concluded that it was probably best to leave it for now. > > I tried it out, and I think it's worth just doing that now, which > is why there's a new 5/6 here: We start by adding an > INDEX_STATE_INIT macro, and corresponding function. > > There's a bit of churn in 6/6 as all of those now will take a > "repo" argument, but I think the end result is worth it, because > even if "repo" remains the only thing that we need to initialize > we're now able to use ALLOC_ARRAY() instead of CALLOC_ARRAY(). > > We'll thus be helped by analysis tools (which would show access to > un-init'd memory) if we miss properly init-ing not just the "repo" > field, but anything in the structure, so our test coverage will be > better. > > It also makes the code easier to follow and change, as it's now > more obvious where we initialize this, and it'll be easier to > change it in the future if e.g. we add a new member that has > mandatory initialization (e.g. a "struct strbuf"). I wasn't expecting the initializer idea to work as well as it has. Now, Patch 5 does all the heavy lifting and Patch 6 is an easy read. Outside of one question about the istate->initialized member (which might not need any change) I'm happy with this version. Thanks, -Stolee
