On 24/07/09 08:35PM, shejialuo wrote:
> Add refs-related options to the "fsck_options", create refs-specific
> "error_func" callback "fsck_refs_error_function".
>
> "fsck_refs_error_function" will use the "oid" parameter. When the caller
> passes the oid, it will use "oid_to_hex" to get the corresponding hex
> value to report to the caller.
Out of curiousity, under what circumstances would the caller want to
also pass the oid? Would it simply be to optionally provide additional
context?
>
> Last, add "FSCK_REFS_OPTIONS_DEFAULT" and "FSCK_REFS_OPTIONS_STRICT"
> macros to create refs options easily.
>
> Mentored-by: Patrick Steinhardt <ps@xxxxxx>
> Mentored-by: Karthik Nayak <karthik.188@xxxxxxxxx>
> Signed-off-by: shejialuo <shejialuo@xxxxxxxxx>
> ---
> fsck.c | 23 +++++++++++++++++++++++
> fsck.h | 15 +++++++++++++++
> 2 files changed, 38 insertions(+)
>
> diff --git a/fsck.c b/fsck.c
> index e1819964e3..c5c7e8454f 100644
> --- a/fsck.c
> +++ b/fsck.c
> @@ -1252,6 +1252,29 @@ int fsck_objects_error_function(struct fsck_options *o,
> return 1;
> }
>
> +int fsck_refs_error_function(struct fsck_options *options UNUSED,
> + const struct object_id *oid,
> + enum object_type object_type UNUSED,
> + const char *checked_ref_name,
> + enum fsck_msg_type msg_type,
> + enum fsck_msg_id msg_id UNUSED,
> + const char *message)
> +{
> + static struct strbuf sb = STRBUF_INIT;
> +
> + strbuf_reset(&sb);
Naive question, is there reason to reset `sb` immediately after
`STRBUF_INIT`? My understanding is that because we initialize the
buffer, the other fields should also be zeroed. If so, resetting the
buffer here seems redundant.
> + strbuf_addstr(&sb, checked_ref_name);
> + if (oid)
> + strbuf_addf(&sb, " -> (%s)", oid_to_hex(oid));
> +
> + if (msg_type == FSCK_WARN) {
> + warning("%s: %s", sb.buf, message);
> + return 0;
> + }
> + error("%s: %s", sb.buf, message);
> + return 1;
> +}
> +
> static int fsck_blobs(struct oidset *blobs_found, struct oidset *blobs_done,
> enum fsck_msg_id msg_missing, enum fsck_msg_id msg_type,
> struct fsck_options *options, const char *blob_type)
> diff --git a/fsck.h b/fsck.h
> index 8ce48395f6..ff52913494 100644
> --- a/fsck.h
> +++ b/fsck.h
> @@ -135,11 +135,19 @@ int fsck_objects_error_cb_print_missing_gitmodules(struct fsck_options *o,
> enum fsck_msg_type msg_type,
> enum fsck_msg_id msg_id,
> const char *message);
> +int fsck_refs_error_function(struct fsck_options *options,
> + 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;
> fsck_error error_func;
> unsigned strict:1;
> + unsigned verbose_refs:1;
What is the purpose of adding `verbose_refs` in this patch? At this
point, I'm not seeing it used. If there is a reason to be included in
this patch, it might be nice to mention in the commit message.
> enum fsck_msg_type *msg_type;
> struct oidset skip_oids;
> struct oidset gitmodules_found;
> @@ -173,6 +181,13 @@ struct fsck_options {
> .gitattributes_done = OIDSET_INIT, \
> .error_func = fsck_objects_error_cb_print_missing_gitmodules, \
> }
> +#define FSCK_REFS_OPTIONS_DEFAULT { \
> + .error_func = fsck_refs_error_function, \
> +}
> +#define FSCK_REFS_OPTIONS_STRICT { \
> + .strict = 1, \
> + .error_func = fsck_refs_error_function, \
> +}
>
> /* descend in all linked child objects
> * the return value is:
> --
> 2.45.2
>
