Re: [GSoC][PATCH v3 1/7] fsck: add refs check interfaces to interface with fsck error levels

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux