Junio C Hamano <gitster@xxxxxxxxx> writes: > > shejialuo <shejialuo@xxxxxxxxx> writes: > > > On Tue, Jun 18, 2024 at 04:38:07AM -0400, Karthik Nayak wrote: > >> shejialuo <shejialuo@xxxxxxxxx> writes: > >> > >> [snip] > >> > >> > struct fsck_options { > >> > + /* > >> > + * Reorder the fields to allow `fsck_ref_options` to use > >> > + * the interfaces using `struct fsck_options`. > >> > + */ > >> > >> Why is this added? It makes sense to have it in the commit message > >> because it talks about the change, but why make it persistent in the > >> code? > >> > > > > I explicitly add this comments due to the following reasons: > > > > 1. If someone needs to change the `fsck_options`, without this comment, > > he might be just add some new fields at top. Although the change will > > fail the tests here, I think we should mention this in code. > > Do you mean you plan to take advantage of the fact that early > members of two structures are the same? IOW, if there is a function > that takes a pointer to smaller fsck_refs_options, you plan to pass > a pointer to fsck_options from some callers, e.g. > > extern void func(struct fsck_refs_options *); > void a_caller(struct fsck_options *o) > { > func((struct fsck_options *)o); > ... > I do not want to convert "struct fsck_options*" to "struct fsck_refs_options*". Instead, I want to convert "struct fsck_refs_options*" to "struct fsck_options*" to reuse the functions which use the "struct fsck_options*" parameter. Like the commit message said: Move the "msg_type" and "strict" member to the top of the "fsck_options" which allows us to convert "fsck_refs_options *" to "fsck_options *" to reuse the interfaces provided by "fsck.h" without changing the original code. It may seem we should add some fields into `fsck_options`, but I don't think it's a good idea. I will elaborate my design here: The git-fsck(1) is highly relevant with the object database. I don't want to add some new fields into "fsck_options" due to the following reason: The fields in fsck_options are all related to objects except "fsck_msg_type" and "strict". Adding some ref-related fields into "fsck_options" will break the semantics. Actually, it may be perfect that I may abstract some interfaces here, however, it would be too complicated for this patch, many functions use "fsck_options *" as the parameter. I think we may do this later.