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@xxxxxx>
> Mentored-by: Karthik Nayak <karthik.188@xxxxxxxxx>
> Signed-off-by: shejialuo <shejialuo@xxxxxxxxx>
> ---
> 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.
-Justin
