| Message ID | ZovrFCzRg06pq5eI@ArchLinux (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
shejialuo <shejialuo@gmail.com> writes: > 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. > Not sure I really understand why we need to do this. Why can't we simply add `const char *checked_ref_name` to the existing 'report' and propagate this also to 'error_func'. Why do we need all this parallel flows? Apart from that, what does 'v' in 'vfsck_report' signify? Perhaps it is also because this commit is doing a lot of things and we could have simplified it into smaller commits? > 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 | 42 ++++++++++++++++--------- > object-file.c | 11 ++++--- > 5 files changed, 102 insertions(+), 48 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) > { > 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; > > diff --git a/builtin/mktag.c b/builtin/mktag.c > index 4767f1a97e..42f945c584 100644 > --- a/builtin/mktag.c > +++ b/builtin/mktag.c > @@ -20,6 +20,7 @@ static struct fsck_options fsck_options = FSCK_OPTIONS_STRICT; > static int mktag_fsck_error_func(struct fsck_options *o UNUSED, > const struct object_id *oid UNUSED, > enum object_type object_type UNUSED, > + const char *checked_ref_name UNUSED, > enum fsck_msg_type msg_type, > enum fsck_msg_id msg_id UNUSED, > const char *message) > diff --git a/fsck.c b/fsck.c > index 1960bfeba9..7182ce8e80 100644 > --- a/fsck.c > +++ b/fsck.c > @@ -226,12 +226,18 @@ static int object_on_skiplist(struct fsck_options *opts, > return opts && oid && oidset_contains(&opts->oid_skiplist, oid); > } > > -__attribute__((format (printf, 5, 6))) > -static int report(struct fsck_options *options, > - const struct object_id *oid, enum object_type object_type, > - enum fsck_msg_id msg_id, const char *fmt, ...) > +/* > + * Provide a unified interface for either fscking refs or objects. > + * It will get the current msg error type and call the error_func callback > + * which is registered in the "fsck_options" struct. > + */ > +static int vfsck_report(struct fsck_options *options, > + const struct object_id *oid, > + enum object_type object_type, > + const char *checked_ref_name, > + enum fsck_msg_id msg_id, const char *fmt, va_list ap) > { > - va_list ap; > + va_list ap_copy; > struct strbuf sb = STRBUF_INIT; > enum fsck_msg_type msg_type = fsck_msg_type(msg_id, options); > int result; > @@ -250,9 +256,9 @@ static int report(struct fsck_options *options, > prepare_msg_ids(); > strbuf_addf(&sb, "%s: ", msg_id_info[msg_id].camelcased); > > - va_start(ap, fmt); > - strbuf_vaddf(&sb, fmt, ap); > - result = options->error_func(options, oid, object_type, > + va_copy(ap_copy, ap); > + strbuf_vaddf(&sb, fmt, ap_copy); > + result = options->error_func(options, oid, object_type, checked_ref_name, > msg_type, msg_id, sb.buf); > strbuf_release(&sb); > va_end(ap); > @@ -260,6 +266,36 @@ static int report(struct fsck_options *options, > return result; > } > > +__attribute__((format (printf, 5, 6))) > Shouldn't this be moved to the header file too? > +static int report(struct fsck_options *options, > + const struct object_id *oid, enum object_type object_type, > + enum fsck_msg_id msg_id, const char *fmt, ...) > +{ > + va_list ap; > + int result; > + va_start(ap, fmt); > + result = vfsck_report(options, oid, object_type, NULL, > + msg_id, fmt, ap); > + va_end(ap); > + return result; > +} > + > + > + There is an extra newline here. > +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, ...) > +{ > + va_list ap; > + int result; > + va_start(ap, fmt); > + result = vfsck_report(options, oid, OBJ_NONE, > + checked_ref_name, msg_id, fmt, ap); > + va_end(ap); > + return result; > +} > + > void fsck_enable_object_names(struct fsck_options *options) > { > if (!options->object_names) > @@ -1200,12 +1236,13 @@ int fsck_buffer(const struct object_id *oid, enum object_type type, > type); > } > > -int fsck_error_function(struct fsck_options *o, > - const struct object_id *oid, > - enum object_type object_type UNUSED, > - enum fsck_msg_type msg_type, > - enum fsck_msg_id msg_id UNUSED, > - const char *message) > +int fsck_objects_error_function(struct fsck_options *o, > + const struct object_id *oid, > + enum object_type object_type UNUSED, > + const char *checked_ref_name UNUSED, > + enum fsck_msg_type msg_type, > + enum fsck_msg_id msg_id UNUSED, > + const char *message) > { > if (msg_type == FSCK_WARN) { > warning("object %s: %s", fsck_describe_object(o, oid), message); > @@ -1303,16 +1340,18 @@ int git_fsck_config(const char *var, const char *value, > * Custom error callbacks that are used in more than one place. > */ > > -int fsck_error_cb_print_missing_gitmodules(struct fsck_options *o, > - const struct object_id *oid, > - enum object_type object_type, > - enum fsck_msg_type msg_type, > - enum fsck_msg_id msg_id, > - const char *message) > +int fsck_objects_error_cb_print_missing_gitmodules(struct fsck_options *o, > + const struct object_id *oid, > + enum object_type object_type, > + const char *checked_ref_name, > + enum fsck_msg_type msg_type, > + enum fsck_msg_id msg_id, > + const char *message) > { > if (msg_id == FSCK_MSG_GITMODULES_MISSING) { > puts(oid_to_hex(oid)); > return 0; > } > - return fsck_error_function(o, oid, object_type, msg_type, msg_id, message); > + return fsck_objects_error_function(o, oid, object_type, checked_ref_name, > + msg_type, msg_id, message); > } > diff --git a/fsck.h b/fsck.h > index 1ee3dd85ba..f703dfb5e8 100644 > --- a/fsck.h > +++ b/fsck.h > @@ -114,22 +114,27 @@ 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 *checked_ref_name, > enum fsck_msg_type msg_type, enum fsck_msg_id msg_id, > const char *message); > > -int fsck_error_function(struct fsck_options *o, > - const struct object_id *oid, enum object_type object_type, > - enum fsck_msg_type msg_type, enum fsck_msg_id msg_id, > - const char *message); > -int fsck_error_cb_print_missing_gitmodules(struct fsck_options *o, > - const struct object_id *oid, > - enum object_type object_type, > - enum fsck_msg_type msg_type, > - enum fsck_msg_id msg_id, > - const char *message); > +int fsck_objects_error_function(struct fsck_options *o, > + const struct object_id *oid, enum object_type object_type, > + const char *checked_ref_name, > + enum fsck_msg_type msg_type, enum fsck_msg_id msg_id, > + const char *message); > +int fsck_objects_error_cb_print_missing_gitmodules(struct fsck_options *o, > + const struct object_id *oid, > + enum object_type object_type, > + const char *checked_ref_name, > + enum fsck_msg_type msg_type, > + enum fsck_msg_id msg_id, > + const char *message); > > struct fsck_options { > fsck_walk_func walk; > @@ -145,12 +150,12 @@ struct fsck_options { > }; > > #define FSCK_OPTIONS_DEFAULT { \ > - .skiplist = OIDSET_INIT, \ > + .oid_skiplist = OIDSET_INIT, \ > .gitmodules_found = OIDSET_INIT, \ > .gitmodules_done = OIDSET_INIT, \ > .gitattributes_found = OIDSET_INIT, \ > .gitattributes_done = OIDSET_INIT, \ > - .error_func = fsck_error_function \ > + .error_func = fsck_objects_error_function \ > } > #define FSCK_OPTIONS_STRICT { \ > .strict = 1, \ > @@ -158,7 +163,7 @@ struct fsck_options { > .gitmodules_done = OIDSET_INIT, \ > .gitattributes_found = OIDSET_INIT, \ > .gitattributes_done = OIDSET_INIT, \ > - .error_func = fsck_error_function, \ > + .error_func = fsck_objects_error_function, \ > } > #define FSCK_OPTIONS_MISSING_GITMODULES { \ > .strict = 1, \ > @@ -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, ...); > + > /* > * Subsystem for storing human-readable names for each object. > * > diff --git a/object-file.c b/object-file.c > index 065103be3e..d2c6427935 100644 > --- a/object-file.c > +++ b/object-file.c > @@ -2470,11 +2470,12 @@ int repo_has_object_file(struct repository *r, > * give more context. > */ > static int hash_format_check_report(struct fsck_options *opts UNUSED, > - const struct object_id *oid UNUSED, > - enum object_type object_type UNUSED, > - enum fsck_msg_type msg_type UNUSED, > - enum fsck_msg_id msg_id UNUSED, > - const char *message) > + const struct object_id *oid UNUSED, > + enum object_type object_type UNUSED, > + const char *ref_checked_name UNUSED, > + enum fsck_msg_type msg_type UNUSED, > + enum fsck_msg_id msg_id UNUSED, > + const char *message) > { > error(_("object fails fsck: %s"), message); > return 1; > -- > 2.45.2
On Mon, Jul 08, 2024 at 10:36:38AM -0400, Karthik Nayak wrote: > shejialuo <shejialuo@gmail.com> writes: > > > 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. > > > > Not sure I really understand why we need to do this. Why can't we simply > add `const char *checked_ref_name` to the existing 'report' and > propagate this also to 'error_func'. Why do we need all this parallel > flows? > Yes, we could just add a parameter "const char *checked_ref_name" to the existing "report". This may seem the simplest way to do. However, it will also introduce some trouble below: 1. "report" function should be exported to the outside, we need to rename it to "fsck_report". Well, we need to change a lot of code here. And we MUST do this, because "report" is a general name. When exporting to the outside, it's not proper. 2. When we add a new parameter in "report", for all the "report" calls, we need to pass this new parameter with NULL. Use this way, we could do not change "report" function prototype and the corresponding calls. Most importantly, we could let the caller feel transparent. Using "report", caller can just ignore "checked_ref_name". Also for "fsck_refs_report", we could ignore some UNUSED parameters. So I think this design is more elegant than just adding a new parameter in the existing "report" function. > Apart from that, what does 'v' in 'vfsck_report' signify? > Because I use "va_list" parameter, I want to emphasis on this. And this provides flexibility that we could add a "fsck_report" function later. There are many codes in git code base using this way. I just followed this. > Perhaps it is also because this commit is doing a lot of things and we > could have simplified it into smaller commits? > Actually, this commit is very clear. I just want to provide a unified function "vfsck_report" here. And let the "report" use this function and "fsck_refs_report" function use this. So I don't know whether we should split this commit into multiple commits. They are just tied together.
shejialuo <shejialuo@gmail.com> writes: > On Mon, Jul 08, 2024 at 10:36:38AM -0400, Karthik Nayak wrote: >> shejialuo <shejialuo@gmail.com> writes: >> >> > 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. >> > >> >> Not sure I really understand why we need to do this. Why can't we simply >> add `const char *checked_ref_name` to the existing 'report' and >> propagate this also to 'error_func'. Why do we need all this parallel >> flows? >> > > Yes, we could just add a parameter "const char *checked_ref_name" to the > existing "report". This may seem the simplest way to do. However, it > will also introduce some trouble below: > > 1. "report" function should be exported to the outside, we need to > rename it to "fsck_report". Well, we need to change a lot of code here. > And we MUST do this, because "report" is a general name. When exporting > to the outside, it's not proper. > agreed. > 2. When we add a new parameter in "report", for all the "report" calls, > we need to pass this new parameter with NULL. > agreed too. > Use this way, we could do not change "report" function prototype and the > corresponding calls. Most importantly, we could let the caller feel > transparent. Using "report", caller can just ignore "checked_ref_name". > Also for "fsck_refs_report", we could ignore some UNUSED parameters. > > So I think this design is more elegant than just adding a new parameter > in the existing "report" function. > I understand what you're saying, I also checked and can see that there are 60 references to the `report()` function. So perhaps there is some merit in keeping it as is and adding a new 'report_refs()'. >> Apart from that, what does 'v' in 'vfsck_report' signify? >> > > Because I use "va_list" parameter, I want to emphasis on this. And this > provides flexibility that we could add a "fsck_report" function later. > There are many codes in git code base using this way. I just followed > this. > I see. Makes sense. >> Perhaps it is also because this commit is doing a lot of things and we >> could have simplified it into smaller commits? >> > > Actually, this commit is very clear. I just want to provide a unified > function "vfsck_report" here. And let the "report" use this function and > "fsck_refs_report" function use this. > > So I don't know whether we should split this commit into multiple > commits. They are just tied together.
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) { 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; diff --git a/builtin/mktag.c b/builtin/mktag.c index 4767f1a97e..42f945c584 100644 --- a/builtin/mktag.c +++ b/builtin/mktag.c @@ -20,6 +20,7 @@ static struct fsck_options fsck_options = FSCK_OPTIONS_STRICT; static int mktag_fsck_error_func(struct fsck_options *o UNUSED, const struct object_id *oid UNUSED, enum object_type object_type UNUSED, + const char *checked_ref_name UNUSED, enum fsck_msg_type msg_type, enum fsck_msg_id msg_id UNUSED, const char *message) diff --git a/fsck.c b/fsck.c index 1960bfeba9..7182ce8e80 100644 --- a/fsck.c +++ b/fsck.c @@ -226,12 +226,18 @@ static int object_on_skiplist(struct fsck_options *opts, return opts && oid && oidset_contains(&opts->oid_skiplist, oid); } -__attribute__((format (printf, 5, 6))) -static int report(struct fsck_options *options, - const struct object_id *oid, enum object_type object_type, - enum fsck_msg_id msg_id, const char *fmt, ...) +/* + * Provide a unified interface for either fscking refs or objects. + * It will get the current msg error type and call the error_func callback + * which is registered in the "fsck_options" struct. + */ +static int vfsck_report(struct fsck_options *options, + const struct object_id *oid, + enum object_type object_type, + const char *checked_ref_name, + enum fsck_msg_id msg_id, const char *fmt, va_list ap) { - va_list ap; + va_list ap_copy; struct strbuf sb = STRBUF_INIT; enum fsck_msg_type msg_type = fsck_msg_type(msg_id, options); int result; @@ -250,9 +256,9 @@ static int report(struct fsck_options *options, prepare_msg_ids(); strbuf_addf(&sb, "%s: ", msg_id_info[msg_id].camelcased); - va_start(ap, fmt); - strbuf_vaddf(&sb, fmt, ap); - result = options->error_func(options, oid, object_type, + va_copy(ap_copy, ap); + strbuf_vaddf(&sb, fmt, ap_copy); + result = options->error_func(options, oid, object_type, checked_ref_name, msg_type, msg_id, sb.buf); strbuf_release(&sb); va_end(ap); @@ -260,6 +266,36 @@ static int report(struct fsck_options *options, return result; } +__attribute__((format (printf, 5, 6))) +static int report(struct fsck_options *options, + const struct object_id *oid, enum object_type object_type, + enum fsck_msg_id msg_id, const char *fmt, ...) +{ + va_list ap; + int result; + va_start(ap, fmt); + result = vfsck_report(options, oid, object_type, NULL, + msg_id, fmt, ap); + va_end(ap); + return result; +} + + + +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, ...) +{ + va_list ap; + int result; + va_start(ap, fmt); + result = vfsck_report(options, oid, OBJ_NONE, + checked_ref_name, msg_id, fmt, ap); + va_end(ap); + return result; +} + void fsck_enable_object_names(struct fsck_options *options) { if (!options->object_names) @@ -1200,12 +1236,13 @@ int fsck_buffer(const struct object_id *oid, enum object_type type, type); } -int fsck_error_function(struct fsck_options *o, - const struct object_id *oid, - enum object_type object_type UNUSED, - enum fsck_msg_type msg_type, - enum fsck_msg_id msg_id UNUSED, - const char *message) +int fsck_objects_error_function(struct fsck_options *o, + const struct object_id *oid, + enum object_type object_type UNUSED, + const char *checked_ref_name UNUSED, + enum fsck_msg_type msg_type, + enum fsck_msg_id msg_id UNUSED, + const char *message) { if (msg_type == FSCK_WARN) { warning("object %s: %s", fsck_describe_object(o, oid), message); @@ -1303,16 +1340,18 @@ int git_fsck_config(const char *var, const char *value, * Custom error callbacks that are used in more than one place. */ -int fsck_error_cb_print_missing_gitmodules(struct fsck_options *o, - const struct object_id *oid, - enum object_type object_type, - enum fsck_msg_type msg_type, - enum fsck_msg_id msg_id, - const char *message) +int fsck_objects_error_cb_print_missing_gitmodules(struct fsck_options *o, + const struct object_id *oid, + enum object_type object_type, + const char *checked_ref_name, + enum fsck_msg_type msg_type, + enum fsck_msg_id msg_id, + const char *message) { if (msg_id == FSCK_MSG_GITMODULES_MISSING) { puts(oid_to_hex(oid)); return 0; } - return fsck_error_function(o, oid, object_type, msg_type, msg_id, message); + return fsck_objects_error_function(o, oid, object_type, checked_ref_name, + msg_type, msg_id, message); } diff --git a/fsck.h b/fsck.h index 1ee3dd85ba..f703dfb5e8 100644 --- a/fsck.h +++ b/fsck.h @@ -114,22 +114,27 @@ 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 *checked_ref_name, enum fsck_msg_type msg_type, enum fsck_msg_id msg_id, const char *message); -int fsck_error_function(struct fsck_options *o, - const struct object_id *oid, enum object_type object_type, - enum fsck_msg_type msg_type, enum fsck_msg_id msg_id, - const char *message); -int fsck_error_cb_print_missing_gitmodules(struct fsck_options *o, - const struct object_id *oid, - enum object_type object_type, - enum fsck_msg_type msg_type, - enum fsck_msg_id msg_id, - const char *message); +int fsck_objects_error_function(struct fsck_options *o, + const struct object_id *oid, enum object_type object_type, + const char *checked_ref_name, + enum fsck_msg_type msg_type, enum fsck_msg_id msg_id, + const char *message); +int fsck_objects_error_cb_print_missing_gitmodules(struct fsck_options *o, + const struct object_id *oid, + enum object_type object_type, + const char *checked_ref_name, + enum fsck_msg_type msg_type, + enum fsck_msg_id msg_id, + const char *message); struct fsck_options { fsck_walk_func walk; @@ -145,12 +150,12 @@ struct fsck_options { }; #define FSCK_OPTIONS_DEFAULT { \ - .skiplist = OIDSET_INIT, \ + .oid_skiplist = OIDSET_INIT, \ .gitmodules_found = OIDSET_INIT, \ .gitmodules_done = OIDSET_INIT, \ .gitattributes_found = OIDSET_INIT, \ .gitattributes_done = OIDSET_INIT, \ - .error_func = fsck_error_function \ + .error_func = fsck_objects_error_function \ } #define FSCK_OPTIONS_STRICT { \ .strict = 1, \ @@ -158,7 +163,7 @@ struct fsck_options { .gitmodules_done = OIDSET_INIT, \ .gitattributes_found = OIDSET_INIT, \ .gitattributes_done = OIDSET_INIT, \ - .error_func = fsck_error_function, \ + .error_func = fsck_objects_error_function, \ } #define FSCK_OPTIONS_MISSING_GITMODULES { \ .strict = 1, \ @@ -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, ...); + /* * Subsystem for storing human-readable names for each object. * diff --git a/object-file.c b/object-file.c index 065103be3e..d2c6427935 100644 --- a/object-file.c +++ b/object-file.c @@ -2470,11 +2470,12 @@ int repo_has_object_file(struct repository *r, * give more context. */ static int hash_format_check_report(struct fsck_options *opts UNUSED, - const struct object_id *oid UNUSED, - enum object_type object_type UNUSED, - enum fsck_msg_type msg_type UNUSED, - enum fsck_msg_id msg_id UNUSED, - const char *message) + const struct object_id *oid UNUSED, + enum object_type object_type UNUSED, + const char *ref_checked_name UNUSED, + enum fsck_msg_type msg_type UNUSED, + enum fsck_msg_id msg_id UNUSED, + const char *message) { error(_("object fails fsck: %s"), message); return 1;
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 | 42 ++++++++++++++++--------- object-file.c | 11 ++++--- 5 files changed, 102 insertions(+), 48 deletions(-)