Re: [GSoC][PATCH v8 3/9] fsck: add refs-related options and error report function
- From
Karthik Nayak <karthik.188@gmail.com>
- Date
- Jul 8, 2024, 14:49 UTC
- Message-ID
- <CAOLa=ZSgZMmfJWcPdKYyLrO-Mo0G-TKj3pYRREjvLt_ypuEk6g@mail.gmail.com>
- In-Reply-To
- <ZovrI1yRmS0X2BHu@ArchLinux>
shejialuo <shejialuo@gmail.com> writes:
Show 10 quoted lines
> 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. > > Last, add "FSCK_REFS_OPTIONS_DEFAULT" and "FSCK_REFS_OPTIONS_STRICT" > macros to create refs options easily. >
Carrying over from the previous commit, couldn't we simply do something like:
diff --git a/fsck.c b/fsck.c
index eea7145470..32ae36a4fc 100644
--- a/fsck.c
+++ b/fsck.c
@@ -1202,17 +1203,33 @@ int fsck_buffer(const struct object_id
*oid, enum object_type type, int fsck_error_function(struct fsck_options *o,
const struct object_id *oid,
- enum object_type object_type UNUSED,
+ enum object_type object_type,
+ 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;
+ int ret = 0;
+
+ if (checked_ref_name) {
+ strbuf_addstr("ref %s", checked_ref_name);
+ if (oid)
+ strbuf_addf(&sb, " -> (%s)", oid_to_hex(oid));
+ } else {
+ strbuf_addstr("object %s", fsck_describe_object(o, oid));
+ }
+
if (msg_type == FSCK_WARN) {
- warning("object %s: %s", fsck_describe_object(o, oid), message);
- return 0;
+ warning("%s: %s", sb.buf, message);
+ ret = 0;
+ goto cleanup;
}
- error("object %s: %s", fsck_describe_object(o, oid), message);
- return 1;
+ error("%s: %s", sb.buf, message);
+
+cleanup:
+ strbuf_release(&sb);
+ return ret;
}Show 38 quoted lines
> Mentored-by: Patrick Steinhardt <ps@pks.im>
> Mentored-by: Karthik Nayak <karthik.188@gmail.com>
> Signed-off-by: shejialuo <shejialuo@gmail.com>
> ---
> fsck.c | 22 ++++++++++++++++++++++
> fsck.h | 15 +++++++++++++++
> 2 files changed, 37 insertions(+)
>
> diff --git a/fsck.c b/fsck.c
> index 7182ce8e80..d1dcbdcac2 100644
> --- a/fsck.c
> +++ b/fsck.c
> @@ -1252,6 +1252,28 @@ 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_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;
> +}
> +We don't free strbuf here.
Show 24 quoted lines
> 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 f703dfb5e8..246055c0f9 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;Nit: Here we have the subject 'refs' towards the end of the name.
> enum fsck_msg_type *msg_type; > struct oidset oid_skiplist;
but here we have the subject 'oid' at the start of the name. Perhaps we can rename this to 'skip_oids'?
Show 17 quoted lines
> 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