Re: [GSoC][PATCH v13 04/10] fsck: add refs-related error report function

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

 



On Mon, Jul 29, 2024 at 09:26:51PM +0800, shejialuo wrote:
> Add refs-related options to the "fsck_options", create refs-specific
> "error_func" callback "fsck_refs_error_function".

We should have an explanation _why_ we are adding these functions in the
commit message.

> "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.

It is a bit unclear to me what you mean with "create refs options
easily". Do you mean to say that `git refs check` (or whatever this will
be called) will have flags like "--strict"?

> Mentored-by: Patrick Steinhardt <ps@xxxxxx>
> Mentored-by: Karthik Nayak <karthik.188@xxxxxxxxx>
> Signed-off-by: shejialuo <shejialuo@xxxxxxxxx>
> ---
>  fsck.c | 25 +++++++++++++++++++++++++
>  fsck.h | 14 ++++++++++++++
>  2 files changed, 39 insertions(+)
> 
> diff --git a/fsck.c b/fsck.c
> index af61fa90ba..56de29b4c0 100644
> --- a/fsck.c
> +++ b/fsck.c
> @@ -1251,6 +1251,31 @@ int fsck_objects_error_function(struct fsck_options *o,
>  	return 1;
>  }
>  
> +int fsck_refs_error_function(struct fsck_options *options,
> +			     const struct object_id *oid,
> +			     enum object_type object_type UNUSED,
> +			     const struct fsck_refs_info *refs_info,
> +			     enum fsck_msg_type msg_type,
> +			     enum fsck_msg_id msg_id UNUSED,
> +			     const char *message)
> +{
> +	struct strbuf sb = STRBUF_INIT;
> +	int ret = 0;
> +
> +	strbuf_addstr(&sb, refs_info->path);
> +
> +	if (oid)
> +		strbuf_addf(&sb, " -> (%s)", oid_to_hex(oid));

Okay, so we do end up printing the object ID indeed. But wouldn't we
want to potentially do the same with symbolic refs?

Also, would it make more sense to put the `oid` (and potentially the
`referent` when we also handle symbolic refs) into `struct
fsck_refs_info`? Like this, the whole state would be self-contained in
that structure, which would also make my proposal from a preceding
commit more feasible where the subsystem-specific error functions only
get a void pointer to this structure. It would require another
refactoring on top to move the object type and OID into a `struct
fsck_objects_info`, too, but that shouldn't be too involved, I guess.

Patrick

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