Derrick Stolee <stolee@xxxxxxxxx> writes: > On 12/7/2021 12:45 PM, Han-Wen Nienhuys via GitGitGadget wrote: >> From: Han-Wen Nienhuys <hanwen@xxxxxxxxxx> >> >> This reduces the amount of glue code, because we don't need a void pointer or >> vtable within the structure. >> >> The only snag is that reftable_index_record contain a strbuf, so it cannot be >> zero-initialized. To address this, introduce reftable_record_for() to create a >> fresh instance, given a record type. >> >> Thanks to Peff for the suggestion. >> >> Signed-off-by: Han-Wen Nienhuys <hanwen@xxxxxxxxxx> >> --- >> reftable/block.c | 4 +- >> reftable/block_test.c | 22 +++--- >> reftable/generic.c | 35 ++++---- >> reftable/iter.c | 4 +- >> reftable/merged.c | 37 ++++----- >> reftable/pq.c | 3 +- >> reftable/pq_test.c | 31 ++++---- >> reftable/reader.c | 105 ++++++++++++------------ >> reftable/record.c | 176 ++++++++++++++++------------------------- >> reftable/record.h | 45 +++++------ >> reftable/record_test.c | 162 +++++++++++++++++++------------------ >> reftable/writer.c | 46 ++++++----- > > This is a HUGE diff, especially compared to the previous changes > in this series. I recommend splitting this out into its own series > and finding a way to break it down into smaller changes. As the reftable_record structure is used everywhere (and that is why this step has to touch everywhere), I suspect that a reviewable fix in small chunks would be achievable only if we redo the topic that introduces this hierarchy and fix the type at the source, as if the reftable_record structure was a struct with union in it from the beginning, I am afraid. Perhaps reftable_record_for() can be implemented without changing the shape of the underlying reftable_record structure in an earlier step, then all the users of reftable_record instances can be migrated to call it, and then finally the shape of the structure and the implementation of reftable_record_for() can be updated? If that is doable, then the "migrate each users" part can be split purely by size. But (1) I do not know if the first step is even doable, and (2) I am not sure if it is worth going a somewhat roundabout route to get to the same destination in this case. So...
