diff mbox series

[v6,3/9] ref: initialize target name outside of check functions

Message ID ZxZYZy-9deyT6I9a@ArchLinux (mailing list archive)
State New
Headers show
Series [v6,1/9] ref: initialize "fsck_ref_report" with zero | expand

Commit Message

shejialuo Oct. 21, 2024, 1:34 p.m. UTC
We passes "refs_check_dir" to the "files_fsck_refs_name" function which
allows it to create the checked ref name later. However, when we
introduce a new check function, we have to re-calculate the target name.
It's bad for us to do repeat calculation. Instead, we should calculate
it only once and pass the target name to the check functions.

In order not to do repeat calculation, rename "refs_check_dir" to
"target_name". And in "files_fsck_refs_dir", create a new strbuf
"target_name", thus whenever we handle a new target, calculate the
name and call the check functions one by one.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: shejialuo <shejialuo@gmail.com>
---
 refs/files-backend.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

Comments

karthik nayak Oct. 21, 2024, 3:49 p.m. UTC | #1
shejialuo <shejialuo@gmail.com> writes:

> We passes "refs_check_dir" to the "files_fsck_refs_name" function which
> allows it to create the checked ref name later. However, when we
> introduce a new check function, we have to re-calculate the target name.
> It's bad for us to do repeat calculation. Instead, we should calculate
> it only once and pass the target name to the check functions.
>
> In order not to do repeat calculation, rename "refs_check_dir" to
> "target_name". And in "files_fsck_refs_dir", create a new strbuf

Nit: Why `target_name` and not simply `target`?

> "target_name", thus whenever we handle a new target, calculate the
> name and call the check functions one by one.
>
> Mentored-by: Patrick Steinhardt <ps@pks.im>
> Mentored-by: Karthik Nayak <karthik.188@gmail.com>
> Signed-off-by: shejialuo <shejialuo@gmail.com>
> ---
>  refs/files-backend.c | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
>

[snip]
diff mbox series

Patch

diff --git a/refs/files-backend.c b/refs/files-backend.c
index f246c92684..fbfcd1115c 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3501,12 +3501,12 @@  static int files_ref_store_remove_on_disk(struct ref_store *ref_store,
  */
 typedef int (*files_fsck_refs_fn)(struct ref_store *ref_store,
 				  struct fsck_options *o,
-				  const char *refs_check_dir,
+				  const char *target_name,
 				  struct dir_iterator *iter);
 
 static int files_fsck_refs_name(struct ref_store *ref_store UNUSED,
 				struct fsck_options *o,
-				const char *refs_check_dir,
+				const char *target_name,
 				struct dir_iterator *iter)
 {
 	struct strbuf sb = STRBUF_INIT;
@@ -3519,11 +3519,10 @@  static int files_fsck_refs_name(struct ref_store *ref_store UNUSED,
 	if (iter->basename[0] != '.' && ends_with(iter->basename, ".lock"))
 		goto cleanup;
 
-	strbuf_addf(&sb, "%s/%s", refs_check_dir, iter->relative_path);
-	if (check_refname_format(sb.buf, 0)) {
+	if (check_refname_format(target_name, 0)) {
 		struct fsck_ref_report report = { 0 };
 
-		report.path = sb.buf;
+		report.path = target_name;
 		ret = fsck_report_ref(o, &report,
 				      FSCK_MSG_BAD_REF_NAME,
 				      "invalid refname format");
@@ -3539,6 +3538,7 @@  static int files_fsck_refs_dir(struct ref_store *ref_store,
 			       const char *refs_check_dir,
 			       files_fsck_refs_fn *fsck_refs_fn)
 {
+	struct strbuf target_name = STRBUF_INIT;
 	struct strbuf sb = STRBUF_INIT;
 	struct dir_iterator *iter;
 	int iter_status;
@@ -3557,11 +3557,15 @@  static int files_fsck_refs_dir(struct ref_store *ref_store,
 			continue;
 		} else if (S_ISREG(iter->st.st_mode) ||
 			   S_ISLNK(iter->st.st_mode)) {
+			strbuf_reset(&target_name);
+			strbuf_addf(&target_name, "%s/%s", refs_check_dir,
+				    iter->relative_path);
+
 			if (o->verbose)
-				fprintf_ln(stderr, "Checking %s/%s",
-					   refs_check_dir, iter->relative_path);
+				fprintf_ln(stderr, "Checking %s", target_name.buf);
+
 			for (size_t i = 0; fsck_refs_fn[i]; i++) {
-				if (fsck_refs_fn[i](ref_store, o, refs_check_dir, iter))
+				if (fsck_refs_fn[i](ref_store, o, target_name.buf, iter))
 					ret = -1;
 			}
 		} else {
@@ -3578,6 +3582,7 @@  static int files_fsck_refs_dir(struct ref_store *ref_store,
 
 out:
 	strbuf_release(&sb);
+	strbuf_release(&target_name);
 	return ret;
 }