Re: [GSoC][PATCH v8 3/9] fsck: add refs-related options and error report function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



shejialuo <shejialuo@xxxxxxxxx> writes:

> 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;
     }


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

>  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'?

>  	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

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux