Re: [GSoC][PATCH v9 2/9] fsck: add a unified interface for reporting fsck messages

> On 24/07/09 08:35PM, shejialuo wrote:
> > The static function "report" provided by "fsck.c" aims at checking fsck
> > error type and calling the callback "error_func" to report the message.
> > However, "report" function is only related to object database which
> > cannot be reused for refs. In order to provide a unified interface which
> > can report either objects or refs, create a new function "vfsck_report"
> > by adding "checked_ref_name" parameter following the "report" prototype.
> > Instead of using "...", provide "va_list" to allow more flexibility.
> > 
> > Like "report", the "vfsck_report" function will use "error_func"
> > registered in "fsck_options" to report customized messages. Change
> > "error_func" prototype to align with the new "vfsck_report".
> > 
> > Then, change "report" function to use "vfsck_report" to report objects
> > related messages. Add a new function called "fsck_refs_report" to use
> > "vfsck_report" to report refs related messages.
> > 
> > Mentored-by: Patrick Steinhardt <ps@pks.im>
> > Mentored-by: Karthik Nayak <karthik.188@gmail.com>
> > Signed-off-by: shejialuo <shejialuo@gmail.com>
> > ---
> >  builtin/fsck.c  | 15 ++++-----
> >  builtin/mktag.c |  1 +
> >  fsck.c          | 81 ++++++++++++++++++++++++++++++++++++-------------
> >  fsck.h          | 40 +++++++++++++++---------
> >  object-file.c   | 11 ++++---
> >  5 files changed, 101 insertions(+), 47 deletions(-)
> > 
> > diff --git a/builtin/fsck.c b/builtin/fsck.c
> > index d13a226c2e..de34538c4f 100644
> > --- a/builtin/fsck.c
> > +++ b/builtin/fsck.c
> > @@ -89,12 +89,13 @@ static int objerror(struct object *obj, const char *err)
> >  	return -1;
> >  }
> >  
> > -static int fsck_error_func(struct fsck_options *o UNUSED,
> > -			   const struct object_id *oid,
> > -			   enum object_type object_type,
> > -			   enum fsck_msg_type msg_type,
> > -			   enum fsck_msg_id msg_id UNUSED,
> > -			   const char *message)
> > +static int fsck_objects_error_func(struct fsck_options *o UNUSED,
> > +				   const struct object_id *oid,
> > +				   enum object_type object_type,
> > +				   const char *checked_ref_name UNUSED,
> > +				   enum fsck_msg_type msg_type,
> > +				   enum fsck_msg_id msg_id UNUSED,
> > +				   const char *message)
> 
> This is just a suggestion, but I think it would be slightly easier to
> review if the `*_error_func()` renames were done in a separate preceding
> patch. That way the purpose of the renames can also be clearly
> explained.
> 

> >  {
> >  	switch (msg_type) {
> >  	case FSCK_WARN:
> > @@ -938,7 +939,7 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
> >  
> >  	fsck_walk_options.walk = mark_object;
> >  	fsck_obj_options.walk = mark_used;
> > -	fsck_obj_options.error_func = fsck_error_func;
> > +	fsck_obj_options.error_func = fsck_objects_error_func;
> >  	if (check_strict)
> >  		fsck_obj_options.strict = 1;
> >  
> [snip]
> > @@ -166,7 +171,7 @@ struct fsck_options {
> >  	.gitmodules_done = OIDSET_INIT, \
> >  	.gitattributes_found = OIDSET_INIT, \
> >  	.gitattributes_done = OIDSET_INIT, \
> > -	.error_func = fsck_error_cb_print_missing_gitmodules, \
> > +	.error_func = fsck_objects_error_cb_print_missing_gitmodules, \
> >  }
> >  
> >  /* descend in all linked child objects
> > @@ -209,6 +214,13 @@ int fsck_tag_standalone(const struct object_id *oid, const char *buffer,
> >   */
> >  int fsck_finish(struct fsck_options *options);
> >  
> > +__attribute__((format (printf, 5, 6)))
> > +int fsck_refs_report(struct fsck_options *options,
> > +		     const struct object_id *oid,
> > +		     const char *checked_ref_name,
> > +		     enum fsck_msg_id msg_id,
> > +		     const char *fmt, ...);
> > +
> 
> I think I mentioned this in a previous reply, but it was missed. Not a
> big deal, but it might be nice to document `int fsck_refs_report()`
> here.
>