On Thu, Jul 18, 2024 at 06:26:30AM -0700, Karthik Nayak wrote:
> shejialuo <shejialuo@xxxxxxxxx> writes:
>
> > diff --git a/fsck.h b/fsck.h
> > index bcfb2e34cd..61ca38afd6 100644
> > --- a/fsck.h
> > +++ b/fsck.h
> > @@ -114,19 +114,25 @@ int is_valid_msg_type(const char *msg_id, const char *msg_type);
> > typedef int (*fsck_walk_func)(struct object *obj, enum object_type object_type,
> > void *data, struct fsck_options *options);
> >
> > -/* callback for fsck_object, type is FSCK_ERROR or FSCK_WARN */
> > +/*
> > + * callback function for reporting errors when checking either objects or refs
> > + */
> > typedef int (*fsck_error)(struct fsck_options *o,
> > const struct object_id *oid, enum object_type object_type,
> > + const char *ref_checkee, const char *sub_ref_checkee,
>
> This makes me really wonder if this is the best way we can do this? This
> seems to solve for the current situation, but what happens if you want
> to also adding the reftable size or packed-refs size here? Would you
> introduce another field?
>
> would it be better to add a single `const struct *fsck_refs_info`
> instead?
>
> Perhaps something like:
>
> struct fsck_refs_info {
> char *refname;
> union {
> struct {
> ...
> } reftable;
> struct {
> ...
> } files;
> } u;
> }
>
> Of course we can fill in the details as we need them.
>
I agree, we should design an extensible data structure here. I will use
this idea. Because we don't know what we will do in the current time.
However, I think "refname" is not good, instead I decide to use
"ref_checkee", "refname" may let caller think we only check the refname.
However, we need to also check reflog.
