diff mbox series

[GSoC,v12,02/10] fsck: add a unified interface for reporting fsck messages

Message ID ZpuDCc8SZx8e4mLP@ArchLinux (mailing list archive)
State Superseded
Headers show
Series ref consistency check infra setup | expand

Commit Message

shejialuo July 20, 2024, 9:27 a.m. UTC
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 "fsck_vreport"
following the "report" prototype. Instead of using "...", provide
"va_list" to allow more flexibility.

In order to provide an extensible error report for refs, add a new
"fsck_refs_info" structure to incorporate an union for supporting
different checks for files backend and reftable backend. Then
incorporate this structure into the "fsck_options" to reuse the
"error_func" callback.

Then, change "report" function to use "fsck_vreport" to report objects
related messages. Add a new function called "fsck_refs_report" to use
"fsck_vreport" to report refs related messages.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: shejialuo <shejialuo@gmail.com>
---
 fsck.c | 46 +++++++++++++++++++++++++++++++++++++++-------
 fsck.h | 26 +++++++++++++++++++++++++-
 2 files changed, 64 insertions(+), 8 deletions(-)

Comments

Karthik Nayak July 23, 2024, 8:35 a.m. UTC | #1
shejialuo <shejialuo@gmail.com> writes:

[snip]

> diff --git a/fsck.h b/fsck.h
> index bcfb2e34cd..a2ecb39b51 100644
> --- a/fsck.h
> +++ b/fsck.h
> @@ -114,7 +114,9 @@ int is_valid_msg_type(const char *msg_id, const char *msg_type);
>  typedef int (*fsck_walk_func)(struct object *obj, enum object_type object_type,
>  			      void *data, struct fsck_options *options);
>
> -/* callback for fsck_object, type is FSCK_ERROR or FSCK_WARN */
> +/*
> + * callback function for reporting errors when checking either objects or refs
> + */
>  typedef int (*fsck_error)(struct fsck_options *o,
>  			  const struct object_id *oid, enum object_type object_type,
>  			  enum fsck_msg_type msg_type, enum fsck_msg_id msg_id,
> @@ -131,11 +133,24 @@ int fsck_error_cb_print_missing_gitmodules(struct fsck_options *o,
>  					   enum fsck_msg_id msg_id,
>  					   const char *message);
>
> +/*
> + * The information for reporting refs-related error message
> + */
> +struct fsck_refs_info {
> +	char *ref_checkee;
> +	union {
> +		struct {
> +			char *sub_ref_checkee;
> +		} files;
> +	} u;
> +};
> +

When I suggested using a struct, it was to replace sending both
'ref_checkee' and 'sub_ref_checkee' to the error functions. I.e to send
a single struct instead of two different fields. But now, we've included
it in the 'fsck_options' struct, I don't think it should be part of the
options. Because the fsck_options is used beyond the error function
while 'fsck_refs_info' is only needed in the error functions, right?
diff mbox series

Patch

diff --git a/fsck.c b/fsck.c
index 3f32441492..f2417e65fd 100644
--- a/fsck.c
+++ b/fsck.c
@@ -226,12 +226,17 @@  static int object_on_skiplist(struct fsck_options *opts,
 	return opts && oid && oidset_contains(&opts->skip_oids, oid);
 }
 
-__attribute__((format (printf, 5, 6)))
-static int report(struct fsck_options *options,
-		  const struct object_id *oid, enum object_type object_type,
-		  enum fsck_msg_id msg_id, const char *fmt, ...)
+/*
+ * Provide a unified interface for either fscking refs or objects.
+ * It will get the current msg error type and call the error_func callback
+ * which is registered in the "fsck_options" struct.
+ */
+static int fsck_vreport(struct fsck_options *options,
+			const struct object_id *oid,
+			enum object_type object_type,
+			enum fsck_msg_id msg_id, const char *fmt, va_list ap)
 {
-	va_list ap;
+	va_list ap_copy;
 	struct strbuf sb = STRBUF_INIT;
 	enum fsck_msg_type msg_type = fsck_msg_type(msg_id, options);
 	int result;
@@ -250,8 +255,8 @@  static int report(struct fsck_options *options,
 	prepare_msg_ids();
 	strbuf_addf(&sb, "%s: ", msg_id_info[msg_id].camelcased);
 
-	va_start(ap, fmt);
-	strbuf_vaddf(&sb, fmt, ap);
+	va_copy(ap_copy, ap);
+	strbuf_vaddf(&sb, fmt, ap_copy);
 	result = options->error_func(options, oid, object_type,
 				     msg_type, msg_id, sb.buf);
 	strbuf_release(&sb);
@@ -260,6 +265,33 @@  static int report(struct fsck_options *options,
 	return result;
 }
 
+__attribute__((format (printf, 5, 6)))
+static int report(struct fsck_options *options,
+		  const struct object_id *oid, enum object_type object_type,
+		  enum fsck_msg_id msg_id, const char *fmt, ...)
+{
+	va_list ap;
+	int result;
+
+	va_start(ap, fmt);
+	result = fsck_vreport(options, oid, object_type, msg_id, fmt, ap);
+	va_end(ap);
+
+	return result;
+}
+
+int fsck_refs_report(struct fsck_options *options,
+		     const struct object_id *oid,
+		     enum fsck_msg_id msg_id, const char *fmt, ...)
+{
+	va_list ap;
+	int result;
+	va_start(ap, fmt);
+	result = fsck_vreport(options, oid, OBJ_NONE, msg_id, fmt, ap);
+	va_end(ap);
+	return result;
+}
+
 void fsck_enable_object_names(struct fsck_options *options)
 {
 	if (!options->object_names)
diff --git a/fsck.h b/fsck.h
index bcfb2e34cd..a2ecb39b51 100644
--- a/fsck.h
+++ b/fsck.h
@@ -114,7 +114,9 @@  int is_valid_msg_type(const char *msg_id, const char *msg_type);
 typedef int (*fsck_walk_func)(struct object *obj, enum object_type object_type,
 			      void *data, struct fsck_options *options);
 
-/* callback for fsck_object, type is FSCK_ERROR or FSCK_WARN */
+/*
+ * callback function for reporting errors when checking either objects or refs
+ */
 typedef int (*fsck_error)(struct fsck_options *o,
 			  const struct object_id *oid, enum object_type object_type,
 			  enum fsck_msg_type msg_type, enum fsck_msg_id msg_id,
@@ -131,11 +133,24 @@  int fsck_error_cb_print_missing_gitmodules(struct fsck_options *o,
 					   enum fsck_msg_id msg_id,
 					   const char *message);
 
+/*
+ * The information for reporting refs-related error message
+ */
+struct fsck_refs_info {
+	char *ref_checkee;
+	union {
+		struct {
+			char *sub_ref_checkee;
+		} files;
+	} u;
+};
+
 struct fsck_options {
 	fsck_walk_func walk;
 	fsck_error error_func;
 	unsigned strict:1;
 	enum fsck_msg_type *msg_type;
+	struct fsck_refs_info refs_info;
 	struct oidset skip_oids;
 	struct oidset gitmodules_found;
 	struct oidset gitmodules_done;
@@ -209,6 +224,15 @@  int fsck_tag_standalone(const struct object_id *oid, const char *buffer,
  */
 int fsck_finish(struct fsck_options *options);
 
+/*
+ * Report an error or warning for refs.
+ */
+__attribute__((format (printf, 4, 5)))
+int fsck_refs_report(struct fsck_options *options,
+		     const struct object_id *oid,
+		     enum fsck_msg_id msg_id,
+		     const char *fmt, ...);
+
 /*
  * Subsystem for storing human-readable names for each object.
  *