[PATCH 0/8] [GSOC] [RFC] ref-filter: code logic optimization

[PATCH 0/8] [GSOC] [RFC] ref-filter: code logic optimization

[ZheNing Hu via GitGitGadget]

Last time I submitted a very long patch series:
https://lore.kernel.org/git/pull.1016.git.1628842990.gitgitgadget@gmail.com/
My mentor Christian suggested to split the performance optimization part
out, so this patch series used to optimize code logic in ref-filter.

Changes in this patch series:

 1. Remove grab_oid to reduce unnecessary string comparison.
 2. Use atom_type in grab_person() and merge the two for loops.
 3. Remove strlen() and introducing xstrvfmt_len() and xstrfmt_len() to
    reduce strlen() overhead.
 4. Introduction ref_filter_slopbuf[1] to reduce memory allocation.
 5. Add deref to struct used_atom to increase the readability of the code.
 6. Introduction symref_atom_parser() to increase the readability of the
    code.
 7. Use switch/case instread of if/else to increase the readability of the
    code.

ZheNing Hu (8):
  [GSOC] ref-filter: remove grab_oid() function
  [GSOC] ref-filter: merge two for loop in grab_person
  [GSOC] ref-filter: remove strlen from find_subpos
  [GSOC] ref-filter: introducing xstrvfmt_len() and xstrfmt_len()
  [GSOC] ref-filter: introduction ref_filter_slopbuf[1]
  [GSOC] ref-filter: add deref member to struct used_atom
  [GSOC] ref-filter: introduce symref_atom_parser()
  [GSOC] ref-filter: use switch/case instead of if/else

 ref-filter.c | 432 ++++++++++++++++++++++++++++-----------------------
 1 file changed, 237 insertions(+), 195 deletions(-)


base-commit: f000ecbed922c727382651490e75014f003c89ca
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1021%2Fadlternative%2Fref-filter-opt-code-logic-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1021/adlternative/ref-filter-opt-code-logic-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1021
-- 
gitgitgadget

[PATCH 1/8] [GSOC] ref-filter: remove grab_oid() function

[ZheNing Hu via GitGitGadget]

From: ZheNing Hu <adlternative@gmail.com>

Because "atom_type == ATOM_OBJECTNAME" implies the condition
of `starts_with(name, "objectname")`, "atom_type == ATOM_TREE"
implies the condition of `starts_with(name, "tree")`, so the
check for `starts_with(name, field)` in grab_oid() is redundant.

Remove the grab_oid() from ref-filter, to reduce repeated check.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: ZheNing Hu <adlternative@gmail.com>
---
 ref-filter.c | 26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 93ce2a6ef2e..e38046ca141 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1065,16 +1065,6 @@ static const char *do_grab_oid(const char *field, const struct object_id *oid,
 	}
 }
 
-static int grab_oid(const char *name, const char *field, const struct object_id *oid,
-		    struct atom_value *v, struct used_atom *atom)
-{
-	if (starts_with(name, field)) {
-		v->s = xstrdup(do_grab_oid(field, oid, atom));
-		return 1;
-	}
-	return 0;
-}
-
 /* See grab_values */
 static void grab_common_values(struct atom_value *val, int deref, struct expand_data *oi)
 {
@@ -1100,8 +1090,9 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_
 			}
 		} else if (atom_type == ATOM_DELTABASE)
 			v->s = xstrdup(oid_to_hex(&oi->delta_base_oid));
-		else if (atom_type == ATOM_OBJECTNAME && deref)
-			grab_oid(name, "objectname", &oi->oid, v, &used_atom[i]);
+		else if (atom_type == ATOM_OBJECTNAME && deref) {
+			v->s = xstrdup(do_grab_oid("objectname", &oi->oid, &used_atom[i]));
+		}
 	}
 }
 
@@ -1142,9 +1133,10 @@ static void grab_commit_values(struct atom_value *val, int deref, struct object
 			continue;
 		if (deref)
 			name++;
-		if (atom_type == ATOM_TREE &&
-		    grab_oid(name, "tree", get_commit_tree_oid(commit), v, &used_atom[i]))
+		if (atom_type == ATOM_TREE) {
+			v->s = xstrdup(do_grab_oid("tree", get_commit_tree_oid(commit), &used_atom[i]));
 			continue;
+		}
 		if (atom_type == ATOM_NUMPARENT) {
 			v->value = commit_list_count(commit->parents);
 			v->s = xstrfmt("%lu", (unsigned long)v->value);
@@ -1917,9 +1909,9 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 				v->s = xstrdup(buf + 1);
 			}
 			continue;
-		} else if (!deref && atom_type == ATOM_OBJECTNAME &&
-			   grab_oid(name, "objectname", &ref->objectname, v, atom)) {
-				continue;
+		} else if (!deref && atom_type == ATOM_OBJECTNAME) {
+			   v->s = xstrdup(do_grab_oid("objectname", &ref->objectname, atom));
+			   continue;
 		} else if (atom_type == ATOM_HEAD) {
 			if (atom->u.head && !strcmp(ref->refname, atom->u.head))
 				v->s = xstrdup("*");
-- 
gitgitgadget

[PATCH 2/8] [GSOC] ref-filter: merge two for loop in grab_person

[ZheNing Hu via GitGitGadget]

From: ZheNing Hu <adlternative@gmail.com>

grab_person() uses string for atom type comparison, this is very
inefficient. After the introduction of atom_type, we can use
atom_type to represent the atom type.

So modify the parameter type of grab_person() to atom_type,
and take use of the characteristics of ATOM_AUTHOR ==
ATOM_AUTHORNAME - 1 == ATOM_AUTHOREMAIL - 2 ==
ATOM_AUTHORDATE - 3 to check which author atom type is.
ATOM_COMMITTER, ATOM_TAGGER are same too.

At the same time, merge the for loop which handles %(creator*) in
grab_person() into the previous for loop which handles %(author*),
%(committer*),%(tagger*). This can reduce unnecessary traversal
of the used_atom list.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: ZheNing Hu <adlternative@gmail.com>
---
 ref-filter.c | 57 +++++++++++++++++-----------------------------------
 1 file changed, 18 insertions(+), 39 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index e38046ca141..592b8d9bd0a 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1275,63 +1275,42 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam
 }
 
 /* See grab_values */
-static void grab_person(const char *who, struct atom_value *val, int deref, void *buf)
+static void grab_person(enum atom_type type, struct atom_value *val, int deref, void *buf)
 {
 	int i;
+	const char *who = valid_atom[type].name;
 	int wholen = strlen(who);
 	const char *wholine = NULL;
 
 	for (i = 0; i < used_atom_cnt; i++) {
 		const char *name = used_atom[i].name;
+		enum atom_type atom_type = used_atom[i].atom_type;
 		struct atom_value *v = &val[i];
 		if (!!deref != (*name == '*'))
 			continue;
 		if (deref)
 			name++;
-		if (strncmp(who, name, wholen))
-			continue;
-		if (name[wholen] != 0 &&
-		    strcmp(name + wholen, "name") &&
-		    !starts_with(name + wholen, "email") &&
-		    !starts_with(name + wholen, "date"))
+		if ((atom_type < type || atom_type > type + 3) &&
+		    /*
+		    * For a tag or a commit object, if "creator" or "creatordate" is
+		    * requested, do something special.
+		    */
+		    ((atom_type != ATOM_CREATOR && atom_type != ATOM_CREATORDATE) ||
+		     ((atom_type == ATOM_CREATOR || atom_type == ATOM_CREATORDATE) &&
+		      type != ATOM_TAGGER && type != ATOM_COMMITTER)))
 			continue;
 		if (!wholine)
 			wholine = find_wholine(who, wholen, buf);
 		if (!wholine)
 			return; /* no point looking for it */
-		if (name[wholen] == 0)
+		if (atom_type == type || atom_type == ATOM_CREATOR)
 			v->s = copy_line(wholine);
-		else if (!strcmp(name + wholen, "name"))
+		else if (atom_type == type + 1)
 			v->s = copy_name(wholine);
-		else if (starts_with(name + wholen, "email"))
+		else if (atom_type == type + 2)
 			v->s = copy_email(wholine, &used_atom[i]);
-		else if (starts_with(name + wholen, "date"))
-			grab_date(wholine, v, name);
-	}
-
-	/*
-	 * For a tag or a commit object, if "creator" or "creatordate" is
-	 * requested, do something special.
-	 */
-	if (strcmp(who, "tagger") && strcmp(who, "committer"))
-		return; /* "author" for commit object is not wanted */
-	if (!wholine)
-		wholine = find_wholine(who, wholen, buf);
-	if (!wholine)
-		return;
-	for (i = 0; i < used_atom_cnt; i++) {
-		const char *name = used_atom[i].name;
-		enum atom_type atom_type = used_atom[i].atom_type;
-		struct atom_value *v = &val[i];
-		if (!!deref != (*name == '*'))
-			continue;
-		if (deref)
-			name++;
-
-		if (atom_type == ATOM_CREATORDATE)
+		else if (atom_type == type + 3 || atom_type == ATOM_CREATORDATE)
 			grab_date(wholine, v, name);
-		else if (atom_type == ATOM_CREATOR)
-			v->s = copy_line(wholine);
 	}
 }
 
@@ -1520,13 +1499,13 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, s
 	case OBJ_TAG:
 		grab_tag_values(val, deref, obj);
 		grab_sub_body_contents(val, deref, data);
-		grab_person("tagger", val, deref, buf);
+		grab_person(ATOM_TAGGER, val, deref, buf);
 		break;
 	case OBJ_COMMIT:
 		grab_commit_values(val, deref, obj);
 		grab_sub_body_contents(val, deref, data);
-		grab_person("author", val, deref, buf);
-		grab_person("committer", val, deref, buf);
+		grab_person(ATOM_AUTHOR, val, deref, buf);
+		grab_person(ATOM_COMMITTER, val, deref, buf);
 		break;
 	case OBJ_TREE:
 		/* grab_tree_values(val, deref, obj, buf, sz); */
-- 
gitgitgadget

[PATCH 3/8] [GSOC] ref-filter: remove strlen from find_subpos

[ZheNing Hu via GitGitGadget]

From: ZheNing Hu <adlternative@gmail.com>

find_subpos() use strlen() to get the length of the object
content, but in the caller of find_subpos(), grab_sub_body_contents(),
we already get the length of the object content through data->size.
So add a `size_t buf_size` in find_subpos(), and pass data->size to
it. This can avoid unnecessary strlen() overhead.

Signed-off-by: ZheNing Hu <adlternative@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
---
 ref-filter.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 592b8d9bd0a..875c2e4c39c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1314,7 +1314,7 @@ static void grab_person(enum atom_type type, struct atom_value *val, int deref,
 	}
 }
 
-static void find_subpos(const char *buf,
+static void find_subpos(const char *buf, size_t buf_size,
 			const char **sub, size_t *sublen,
 			const char **body, size_t *bodylen,
 			size_t *nonsiglen,
@@ -1322,12 +1322,12 @@ static void find_subpos(const char *buf,
 {
 	struct strbuf payload = STRBUF_INIT;
 	struct strbuf signature = STRBUF_INIT;
+	const char *begin = buf;
 	const char *eol;
-	const char *end = buf + strlen(buf);
 	const char *sigstart;
 
 	/* parse signature first; we might not even have a subject line */
-	parse_signature(buf, end - buf, &payload, &signature);
+	parse_signature(buf, buf_size, &payload, &signature);
 
 	/* skip past header until we hit empty line */
 	while (*buf && *buf != '\n') {
@@ -1340,7 +1340,7 @@ static void find_subpos(const char *buf,
 	while (*buf == '\n')
 		buf++;
 	*sig = strbuf_detach(&signature, siglen);
-	sigstart = buf + parse_signed_buffer(buf, strlen(buf));
+	sigstart = buf + parse_signed_buffer(buf, buf_size - (buf - begin));
 
 	/* subject is first non-empty line */
 	*sub = buf;
@@ -1363,7 +1363,7 @@ static void find_subpos(const char *buf,
 	while (*buf == '\n' || *buf == '\r')
 		buf++;
 	*body = buf;
-	*bodylen = strlen(buf);
+	*bodylen = buf_size - (buf - begin);
 	*nonsiglen = sigstart - buf;
 }
 
@@ -1430,7 +1430,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct exp
 		     !starts_with(name, "contents")))
 			continue;
 		if (!subpos)
-			find_subpos(buf,
+			find_subpos(buf, data->size,
 				    &subpos, &sublen,
 				    &bodypos, &bodylen, &nonsiglen,
 				    &sigpos, &siglen);
-- 
gitgitgadget

[PATCH 4/8] [GSOC] ref-filter: introducing xstrvfmt_len() and xstrfmt_len()

[ZheNing Hu via GitGitGadget]

From: ZheNing Hu <adlternative@gmail.com>

In the logic of ref-filter, if we already specified v->s_size (the
length of the data), we can avoid using strlen() to calculate the
length of the data in strbuf_addstr() or perl_quote_buf_with_len()...

xstrfmt(), xstrvfmt(), they are functions used to add format strings
to a dynamic allocated buffer. Because their implementation uses strbuf
to store its buffer and its length, so we can easily pass its length
to the caller, furthermore, in ref-filter, we can reduce the use of
strlen().

So introduce xstrvfmt_len() and xstrfmt_len() which can fetch the length
of the buffer through the parameter `len`. This can bring a slight
performance improvement.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: ZheNing Hu <adlternative@gmail.com>
---
 ref-filter.c | 73 ++++++++++++++++++++++++++++++++++------------------
 1 file changed, 48 insertions(+), 25 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 875c2e4c39c..3784c0af57c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -228,6 +228,29 @@ static int strbuf_addf_ret(struct strbuf *sb, int ret, const char *fmt, ...)
 	return ret;
 }
 
+__attribute__((format (printf, 2, 0)))
+static char *xstrvfmt_len(ssize_t *len, const char *fmt, va_list ap)
+{
+	struct strbuf buf = STRBUF_INIT;
+	strbuf_vaddf(&buf, fmt, ap);
+	if (len)
+		*len = buf.len;
+	return strbuf_detach(&buf, NULL);
+}
+
+__attribute__((format (printf, 2, 3)))
+static  char *xstrfmt_len(ssize_t *len, const char *fmt, ...)
+{
+	va_list ap;
+	char *ret;
+
+	va_start(ap, fmt);
+	ret = xstrvfmt_len(len ,fmt, ap);
+	va_end(ap);
+
+	return ret;
+}
+
 static int color_atom_parser(struct ref_format *format, struct used_atom *atom,
 			     const char *color_value, struct strbuf *err)
 {
@@ -1083,10 +1106,10 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_
 		else if (atom_type == ATOM_OBJECTSIZE) {
 			if (used_atom[i].u.objectsize.option == O_SIZE_DISK) {
 				v->value = oi->disk_size;
-				v->s = xstrfmt("%"PRIuMAX, (uintmax_t)oi->disk_size);
+				v->s = xstrfmt_len(&v->s_size, "%"PRIuMAX, (uintmax_t)oi->disk_size);
 			} else if (used_atom[i].u.objectsize.option == O_SIZE) {
 				v->value = oi->size;
-				v->s = xstrfmt("%"PRIuMAX , (uintmax_t)oi->size);
+				v->s = xstrfmt_len(&v->s_size, "%"PRIuMAX , (uintmax_t)oi->size);
 			}
 		} else if (atom_type == ATOM_DELTABASE)
 			v->s = xstrdup(oid_to_hex(&oi->delta_base_oid));
@@ -1139,7 +1162,7 @@ static void grab_commit_values(struct atom_value *val, int deref, struct object
 		}
 		if (atom_type == ATOM_NUMPARENT) {
 			v->value = commit_list_count(commit->parents);
-			v->s = xstrfmt("%lu", (unsigned long)v->value);
+			v->s = xstrfmt_len(&v->s_size, "%lu", (unsigned long)v->value);
 		}
 		else if (atom_type == ATOM_PARENT) {
 			struct commit_list *parents;
@@ -1417,7 +1440,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct exp
 				v->s = xmemdupz(buf, buf_size);
 				v->s_size = buf_size;
 			} else if (atom->u.raw_data.option == RAW_LENGTH) {
-				v->s = xstrfmt("%"PRIuMAX, (uintmax_t)buf_size);
+				v->s = xstrfmt_len(&v->s_size, "%"PRIuMAX, (uintmax_t)buf_size);
 			}
 			continue;
 		}
@@ -1444,7 +1467,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct exp
 		} else if (atom->u.contents.option == C_BODY_DEP)
 			v->s = xmemdupz(bodypos, bodylen);
 		else if (atom->u.contents.option == C_LENGTH)
-			v->s = xstrfmt("%"PRIuMAX, (uintmax_t)strlen(subpos));
+			v->s = xstrfmt_len(&v->s_size, "%"PRIuMAX, (uintmax_t)strlen(subpos));
 		else if (atom->u.contents.option == C_BODY)
 			v->s = xmemdupz(bodypos, nonsiglen);
 		else if (atom->u.contents.option == C_SIG)
@@ -1611,56 +1634,56 @@ static const char *show_ref(struct refname_atom *atom, const char *refname)
 }
 
 static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
-				    struct branch *branch, const char **s)
+				    struct branch *branch, struct atom_value *v)
 {
 	int num_ours, num_theirs;
 	if (atom->u.remote_ref.option == RR_REF)
-		*s = show_ref(&atom->u.remote_ref.refname, refname);
+		v->s = show_ref(&atom->u.remote_ref.refname, refname);
 	else if (atom->u.remote_ref.option == RR_TRACK) {
 		if (stat_tracking_info(branch, &num_ours, &num_theirs,
 				       NULL, atom->u.remote_ref.push,
 				       AHEAD_BEHIND_FULL) < 0) {
-			*s = xstrdup(msgs.gone);
+			v->s = xstrdup(msgs.gone);
 		} else if (!num_ours && !num_theirs)
-			*s = xstrdup("");
+			v->s = xstrdup("");
 		else if (!num_ours)
-			*s = xstrfmt(msgs.behind, num_theirs);
+			v->s = xstrfmt_len(&v->s_size, msgs.behind, num_theirs);
 		else if (!num_theirs)
-			*s = xstrfmt(msgs.ahead, num_ours);
+			v->s = xstrfmt_len(&v->s_size, msgs.ahead, num_ours);
 		else
-			*s = xstrfmt(msgs.ahead_behind,
+			v->s= xstrfmt_len(&v->s_size, msgs.ahead_behind,
 				     num_ours, num_theirs);
-		if (!atom->u.remote_ref.nobracket && *s[0]) {
-			const char *to_free = *s;
-			*s = xstrfmt("[%s]", *s);
+		if (!atom->u.remote_ref.nobracket && v->s[0]) {
+			const char *to_free = v->s;
+			v->s = xstrfmt_len(&v->s_size, "[%s]", v->s);
 			free((void *)to_free);
 		}
 	} else if (atom->u.remote_ref.option == RR_TRACKSHORT) {
 		if (stat_tracking_info(branch, &num_ours, &num_theirs,
 				       NULL, atom->u.remote_ref.push,
 				       AHEAD_BEHIND_FULL) < 0) {
-			*s = xstrdup("");
+			v->s = xstrdup("");
 			return;
 		}
 		if (!num_ours && !num_theirs)
-			*s = xstrdup("=");
+			v->s = xstrdup("=");
 		else if (!num_ours)
-			*s = xstrdup("<");
+			v->s = xstrdup("<");
 		else if (!num_theirs)
-			*s = xstrdup(">");
+			v->s = xstrdup(">");
 		else
-			*s = xstrdup("<>");
+			v->s = xstrdup("<>");
 	} else if (atom->u.remote_ref.option == RR_REMOTE_NAME) {
 		int explicit;
 		const char *remote = atom->u.remote_ref.push ?
 			pushremote_for_branch(branch, &explicit) :
 			remote_for_branch(branch, &explicit);
-		*s = xstrdup(explicit ? remote : "");
+		v->s = xstrdup(explicit ? remote : "");
 	} else if (atom->u.remote_ref.option == RR_REMOTE_REF) {
 		const char *merge;
 
 		merge = remote_ref_for_branch(branch, atom->u.remote_ref.push);
-		*s = xstrdup(merge ? merge : "");
+		v->s = xstrdup(merge ? merge : "");
 	} else
 		BUG("unhandled RR_* enum");
 }
@@ -1849,7 +1872,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 
 			refname = branch_get_upstream(branch, NULL);
 			if (refname)
-				fill_remote_ref_details(atom, refname, branch, &v->s);
+				fill_remote_ref_details(atom, refname, branch, v);
 			else
 				v->s = xstrdup("");
 			continue;
@@ -1870,7 +1893,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 			}
 			/* We will definitely re-init v->s on the next line. */
 			free((char *)v->s);
-			fill_remote_ref_details(atom, refname, branch, &v->s);
+			fill_remote_ref_details(atom, refname, branch, v);
 			continue;
 		} else if (atom_type == ATOM_COLOR) {
 			v->s = xstrdup(atom->u.color);
@@ -1933,7 +1956,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 		if (!deref)
 			v->s = xstrdup(refname);
 		else
-			v->s = xstrfmt("%s^{}", refname);
+			v->s = xstrfmt_len(&v->s_size, "%s^{}", refname);
 		free((char *)refname);
 	}
 
-- 
gitgitgadget

[PATCH 5/8] [GSOC] ref-filter: introduction ref_filter_slopbuf[1]

[ZheNing Hu via GitGitGadget]

From: ZheNing Hu <adlternative@gmail.com>

The ref-filter code uses xstrdup("") extensively to indicate null
value.

xstrdup("") calls strdup(""), which in glibc will call malloc(1),
this means using xstrdup("") will cause unnecessary memory allocation.

Introduce the global one-byte array variable ref_filter_slopbuf[1],
which learn from strbuf_slopbuf[1] in strbuf.c. Using ref_filter_slopbuf
instead of xstrdup(""), we can reduce some malloc/free overhead.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: ZheNing Hu <adlternative@gmail.com>
---
 ref-filter.c | 53 ++++++++++++++++++++++++++--------------------------
 1 file changed, 27 insertions(+), 26 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 3784c0af57c..b74fcbb4806 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -24,6 +24,8 @@
 #include "hashmap.h"
 #include "strvec.h"
 
+static char ref_filter_slopbuf[1];
+
 static struct ref_msg {
 	const char *gone;
 	const char *ahead;
@@ -1209,7 +1211,7 @@ static const char *copy_name(const char *buf)
 		if (!strncmp(cp, " <", 2))
 			return xmemdupz(buf, cp - buf);
 	}
-	return xstrdup("");
+	return ref_filter_slopbuf;
 }
 
 static const char *copy_email(const char *buf, struct used_atom *atom)
@@ -1217,7 +1219,7 @@ static const char *copy_email(const char *buf, struct used_atom *atom)
 	const char *email = strchr(buf, '<');
 	const char *eoemail;
 	if (!email)
-		return xstrdup("");
+		return ref_filter_slopbuf;
 	switch (atom->u.email_option.option) {
 	case EO_RAW:
 		eoemail = strchr(email, '>');
@@ -1239,7 +1241,7 @@ static const char *copy_email(const char *buf, struct used_atom *atom)
 	}
 
 	if (!eoemail)
-		return xstrdup("");
+		return ref_filter_slopbuf;
 	return xmemdupz(email, eoemail - email);
 }
 
@@ -1293,7 +1295,7 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam
 	v->value = timestamp;
 	return;
  bad:
-	v->s = xstrdup("");
+	v->s = ref_filter_slopbuf;
 	v->value = 0;
 }
 
@@ -1503,7 +1505,7 @@ static void fill_missing_values(struct atom_value *val)
 	for (i = 0; i < used_atom_cnt; i++) {
 		struct atom_value *v = &val[i];
 		if (v->s == NULL)
-			v->s = xstrdup("");
+			v->s = ref_filter_slopbuf;
 	}
 }
 
@@ -1576,7 +1578,7 @@ static const char *lstrip_ref_components(const char *refname, int len)
 		switch (*start++) {
 		case '\0':
 			free((char *)to_free);
-			return xstrdup("");
+			return ref_filter_slopbuf;
 		case '/':
 			remaining--;
 			break;
@@ -1614,7 +1616,7 @@ static const char *rstrip_ref_components(const char *refname, int len)
 		char *p = strrchr(start, '/');
 		if (p == NULL) {
 			free((char *)to_free);
-			return xstrdup("");
+			return ref_filter_slopbuf;
 		} else
 			p[0] = '\0';
 	}
@@ -1645,7 +1647,7 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
 				       AHEAD_BEHIND_FULL) < 0) {
 			v->s = xstrdup(msgs.gone);
 		} else if (!num_ours && !num_theirs)
-			v->s = xstrdup("");
+			v->s = ref_filter_slopbuf;
 		else if (!num_ours)
 			v->s = xstrfmt_len(&v->s_size, msgs.behind, num_theirs);
 		else if (!num_theirs)
@@ -1662,7 +1664,7 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
 		if (stat_tracking_info(branch, &num_ours, &num_theirs,
 				       NULL, atom->u.remote_ref.push,
 				       AHEAD_BEHIND_FULL) < 0) {
-			v->s = xstrdup("");
+			v->s = ref_filter_slopbuf;
 			return;
 		}
 		if (!num_ours && !num_theirs)
@@ -1721,7 +1723,7 @@ char *get_head_description(void)
 static const char *get_symref(struct used_atom *atom, struct ref_array_item *ref)
 {
 	if (!ref->symref)
-		return xstrdup("");
+		return ref_filter_slopbuf;
 	else
 		return show_ref(&atom->u.refname, ref->symref);
 }
@@ -1805,7 +1807,7 @@ static char *get_worktree_path(const struct used_atom *atom, const struct ref_ar
 	e = hashmap_get(&(ref_to_worktree_map.map), &entry, ref->refname);
 
 	if (!e)
-		return xstrdup("");
+		return ref_filter_slopbuf;
 
 	lookup_result = container_of(e, struct ref_to_worktree_entry, ent);
 
@@ -1827,7 +1829,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 		ref->symref = resolve_refdup(ref->refname, RESOLVE_REF_READING,
 					     NULL, NULL);
 		if (!ref->symref)
-			ref->symref = xstrdup("");
+			ref->symref = ref_filter_slopbuf;
 	}
 
 	/* Fill in specials first */
@@ -1855,7 +1857,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 			if (ref->kind == FILTER_REFS_BRANCHES)
 				v->s = get_worktree_path(atom, ref);
 			else
-				v->s = xstrdup("");
+				v->s = ref_filter_slopbuf;
 			continue;
 		}
 		else if (atom_type == ATOM_SYMREF)
@@ -1865,7 +1867,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 			/* only local branches may have an upstream */
 			if (!skip_prefix(ref->refname, "refs/heads/",
 					 &branch_name)) {
-				v->s = xstrdup("");
+				v->s = ref_filter_slopbuf;
 				continue;
 			}
 			branch = branch_get(branch_name);
@@ -1874,11 +1876,11 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 			if (refname)
 				fill_remote_ref_details(atom, refname, branch, v);
 			else
-				v->s = xstrdup("");
+				v->s = ref_filter_slopbuf;
 			continue;
 		} else if (atom_type == ATOM_PUSH && atom->u.remote_ref.push) {
 			const char *branch_name;
-			v->s = xstrdup("");
+			v->s = ref_filter_slopbuf;
 			if (!skip_prefix(ref->refname, "refs/heads/",
 					 &branch_name))
 				continue;
@@ -1891,8 +1893,6 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 				if (!refname)
 					continue;
 			}
-			/* We will definitely re-init v->s on the next line. */
-			free((char *)v->s);
 			fill_remote_ref_details(atom, refname, branch, v);
 			continue;
 		} else if (atom_type == ATOM_COLOR) {
@@ -1905,7 +1905,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 			if (ref->flag & REF_ISPACKED)
 				cp = copy_advance(cp, ",packed");
 			if (cp == buf)
-				v->s = xstrdup("");
+				v->s = ref_filter_slopbuf;
 			else {
 				*cp = '\0';
 				v->s = xstrdup(buf + 1);
@@ -1922,33 +1922,33 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 			continue;
 		} else if (atom_type == ATOM_ALIGN) {
 			v->handler = align_atom_handler;
-			v->s = xstrdup("");
+			v->s = ref_filter_slopbuf;
 			continue;
 		} else if (atom_type == ATOM_END) {
 			v->handler = end_atom_handler;
-			v->s = xstrdup("");
+			v->s = ref_filter_slopbuf;
 			continue;
 		} else if (atom_type == ATOM_IF) {
 			const char *s;
 			if (skip_prefix(name, "if:", &s))
 				v->s = xstrdup(s);
 			else
-				v->s = xstrdup("");
+				v->s = ref_filter_slopbuf;
 			v->handler = if_atom_handler;
 			continue;
 		} else if (atom_type == ATOM_THEN) {
 			v->handler = then_atom_handler;
-			v->s = xstrdup("");
+			v->s = ref_filter_slopbuf;
 			continue;
 		} else if (atom_type == ATOM_ELSE) {
 			v->handler = else_atom_handler;
-			v->s = xstrdup("");
+			v->s = ref_filter_slopbuf;
 			continue;
 		} else if (atom_type == ATOM_REST) {
 			if (ref->rest)
 				v->s = xstrdup(ref->rest);
 			else
-				v->s = xstrdup("");
+				v->s = ref_filter_slopbuf;
 			continue;
 		} else
 			continue;
@@ -2297,7 +2297,8 @@ static void free_array_item(struct ref_array_item *item)
 	if (item->value) {
 		int i;
 		for (i = 0; i < used_atom_cnt; i++)
-			free((char *)item->value[i].s);
+			if (item->value[i].s != ref_filter_slopbuf)
+				free((char *)item->value[i].s);
 		free(item->value);
 	}
 	free(item);
-- 
gitgitgadget

[PATCH 6/8] [GSOC] ref-filter: add deref member to struct used_atom

[ZheNing Hu via GitGitGadget]

From: ZheNing Hu <adlternative@gmail.com>

In the ref-filter logic, the dereference flag '*' is stored
at the beginning of used_atom[i].name (e.g. '*objectname:short'),
this leads us to make some check on used_atom[i].name[0] to
know if the atom need to be dereferenced. This is not only
cumbersome to use, but also poor in readability.

So add `deref` flag to struct used_atom used to indicate
whether the atom needs to be dereferenced and let used_atom
record only the rest part (i.e. 'objectname:short').

This can make the logic of the program clearer and easy to use.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: ZheNing Hu <adlternative@gmail.com>
---
 ref-filter.c | 55 ++++++++++++++++++++--------------------------------
 1 file changed, 21 insertions(+), 34 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index b74fcbb4806..5abcbbf10ac 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -173,6 +173,7 @@ enum atom_type {
  * array.
  */
 static struct used_atom {
+	unsigned int deref : 1;
 	enum atom_type atom_type;
 	const char *name;
 	cmp_type type;
@@ -343,7 +344,7 @@ static int objecttype_atom_parser(struct ref_format *format, struct used_atom *a
 {
 	if (arg)
 		return strbuf_addf_ret(err, -1, _("%%(objecttype) does not take arguments"));
-	if (*atom->name == '*')
+	if (atom->deref)
 		oi_deref.info.typep = &oi_deref.type;
 	else
 		oi.info.typep = &oi.type;
@@ -355,13 +356,13 @@ static int objectsize_atom_parser(struct ref_format *format, struct used_atom *a
 {
 	if (!arg) {
 		atom->u.objectsize.option = O_SIZE;
-		if (*atom->name == '*')
+		if (atom->deref)
 			oi_deref.info.sizep = &oi_deref.size;
 		else
 			oi.info.sizep = &oi.size;
 	} else if (!strcmp(arg, "disk")) {
 		atom->u.objectsize.option = O_SIZE_DISK;
-		if (*atom->name == '*')
+		if (atom->deref)
 			oi_deref.info.disk_sizep = &oi_deref.disk_size;
 		else
 			oi.info.disk_sizep = &oi.disk_size;
@@ -375,7 +376,7 @@ static int deltabase_atom_parser(struct ref_format *format, struct used_atom *at
 {
 	if (arg)
 		return strbuf_addf_ret(err, -1, _("%%(deltabase) does not take arguments"));
-	if (*atom->name == '*')
+	if (atom->deref)
 		oi_deref.info.delta_base_oid = &oi_deref.delta_base_oid;
 	else
 		oi.info.delta_base_oid = &oi.delta_base_oid;
@@ -697,10 +698,13 @@ static int parse_ref_filter_atom(struct ref_format *format,
 	const char *sp;
 	const char *arg;
 	int i, at, atom_len;
+	int deref = 0;
 
 	sp = atom;
-	if (*sp == '*' && sp < ep)
+	if (*sp == '*' && sp < ep) {
 		sp++; /* deref */
+		deref = 1;
+	}
 	if (ep <= sp)
 		return strbuf_addf_ret(err, -1, _("malformed field name: %.*s"),
 				       (int)(ep-atom), atom);
@@ -717,7 +721,7 @@ static int parse_ref_filter_atom(struct ref_format *format,
 	/* Do we have the atom already used elsewhere? */
 	for (i = 0; i < used_atom_cnt; i++) {
 		int len = strlen(used_atom[i].name);
-		if (len == ep - atom && !memcmp(used_atom[i].name, atom, len))
+		if (len == ep - sp && !memcmp(used_atom[i].name, sp, len))
 			return i;
 	}
 
@@ -741,17 +745,18 @@ static int parse_ref_filter_atom(struct ref_format *format,
 	used_atom_cnt++;
 	REALLOC_ARRAY(used_atom, used_atom_cnt);
 	used_atom[at].atom_type = i;
-	used_atom[at].name = xmemdupz(atom, ep - atom);
+	used_atom[at].deref = deref;
+	used_atom[at].name = xmemdupz(sp, ep - sp);
 	used_atom[at].type = valid_atom[i].cmp_type;
 	used_atom[at].source = valid_atom[i].source;
 	if (used_atom[at].source == SOURCE_OBJ) {
-		if (*atom == '*')
+		if (deref)
 			oi_deref.info.contentp = &oi_deref.content;
 		else
 			oi.info.contentp = &oi.content;
 	}
 	if (arg) {
-		arg = used_atom[at].name + (arg - atom) + 1;
+		arg = used_atom[at].name + (arg - sp) + 1;
 		if (!*arg) {
 			/*
 			 * Treat empty sub-arguments list as NULL (i.e.,
@@ -763,7 +768,7 @@ static int parse_ref_filter_atom(struct ref_format *format,
 	memset(&used_atom[at].u, 0, sizeof(used_atom[at].u));
 	if (valid_atom[i].parser && valid_atom[i].parser(format, &used_atom[at], arg, err))
 		return -1;
-	if (*atom == '*')
+	if (deref)
 		need_tagged = 1;
 	if (i == ATOM_SYMREF)
 		need_symref = 1;
@@ -1096,13 +1101,10 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_
 	int i;
 
 	for (i = 0; i < used_atom_cnt; i++) {
-		const char *name = used_atom[i].name;
 		enum atom_type atom_type = used_atom[i].atom_type;
 		struct atom_value *v = &val[i];
-		if (!!deref != (*name == '*'))
+		if (!!deref != used_atom[i].deref)
 			continue;
-		if (deref)
-			name++;
 		if (atom_type == ATOM_OBJECTTYPE)
 			v->s = xstrdup(type_name(oi->type));
 		else if (atom_type == ATOM_OBJECTSIZE) {
@@ -1128,13 +1130,10 @@ static void grab_tag_values(struct atom_value *val, int deref, struct object *ob
 	struct tag *tag = (struct tag *) obj;
 
 	for (i = 0; i < used_atom_cnt; i++) {
-		const char *name = used_atom[i].name;
 		enum atom_type atom_type = used_atom[i].atom_type;
 		struct atom_value *v = &val[i];
-		if (!!deref != (*name == '*'))
+		if (!!deref != used_atom[i].deref)
 			continue;
-		if (deref)
-			name++;
 		if (atom_type == ATOM_TAG)
 			v->s = xstrdup(tag->tag);
 		else if (atom_type == ATOM_TYPE && tag->tagged)
@@ -1151,13 +1150,10 @@ static void grab_commit_values(struct atom_value *val, int deref, struct object
 	struct commit *commit = (struct commit *) obj;
 
 	for (i = 0; i < used_atom_cnt; i++) {
-		const char *name = used_atom[i].name;
 		enum atom_type atom_type = used_atom[i].atom_type;
 		struct atom_value *v = &val[i];
-		if (!!deref != (*name == '*'))
+		if (!!deref != used_atom[i].deref)
 			continue;
-		if (deref)
-			name++;
 		if (atom_type == ATOM_TREE) {
 			v->s = xstrdup(do_grab_oid("tree", get_commit_tree_oid(commit), &used_atom[i]));
 			continue;
@@ -1311,10 +1307,8 @@ static void grab_person(enum atom_type type, struct atom_value *val, int deref,
 		const char *name = used_atom[i].name;
 		enum atom_type atom_type = used_atom[i].atom_type;
 		struct atom_value *v = &val[i];
-		if (!!deref != (*name == '*'))
+		if (!!deref != used_atom[i].deref)
 			continue;
-		if (deref)
-			name++;
 		if ((atom_type < type || atom_type > type + 3) &&
 		    /*
 		    * For a tag or a commit object, if "creator" or "creatordate" is
@@ -1430,10 +1424,8 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct exp
 		struct atom_value *v = &val[i];
 		enum atom_type atom_type = atom->atom_type;
 
-		if (!!deref != (*name == '*'))
+		if (!!deref != used_atom[i].deref)
 			continue;
-		if (deref)
-			name++;
 
 		if (atom_type == ATOM_RAW) {
 			unsigned long buf_size = data->size;
@@ -1838,7 +1830,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 		enum atom_type atom_type = atom->atom_type;
 		const char *name = used_atom[i].name;
 		struct atom_value *v = &ref->value[i];
-		int deref = 0;
+		int deref = atom->deref;
 		const char *refname;
 		struct branch *branch = NULL;
 
@@ -1846,11 +1838,6 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 		v->handler = append_atom;
 		v->atom = atom;
 
-		if (*name == '*') {
-			deref = 1;
-			name++;
-		}
-
 		if (atom_type == ATOM_REFNAME)
 			refname = get_refname(atom, ref);
 		else if (atom_type == ATOM_WORKTREEPATH) {
-- 
gitgitgadget

[PATCH 7/8] [GSOC] ref-filter: introduce symref_atom_parser()

[ZheNing Hu via GitGitGadget]

From: ZheNing Hu <adlternative@gmail.com>

parse_ref_filter_atom() sets need_symref by checking
whether the atom type is ATOM_SYMREF. When we are operating
other atoms here, this step of checking is not necessary.

So add the symref_atom_parser() specifically to parse the
%(symref) atom, and set need_symref in it.

This can make the program logic more concise.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: ZheNing Hu <adlternative@gmail.com>
---
 ref-filter.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 5abcbbf10ac..f1c82e20e3d 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -503,6 +503,13 @@ static int person_email_atom_parser(struct ref_format *format, struct used_atom
 	return 0;
 }
 
+static int symref_atom_parser(struct ref_format *format, struct used_atom *atom,
+			       const char *arg, struct strbuf *err)
+{
+	need_symref = 1;
+	return refname_atom_parser_internal(&atom->u.refname, arg, atom->name, err);
+}
+
 static int refname_atom_parser(struct ref_format *format, struct used_atom *atom,
 			       const char *arg, struct strbuf *err)
 {
@@ -642,7 +649,7 @@ static struct {
 	[ATOM_RAW] = { "raw", SOURCE_OBJ, FIELD_STR, raw_atom_parser },
 	[ATOM_UPSTREAM] = { "upstream", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },
 	[ATOM_PUSH] = { "push", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },
-	[ATOM_SYMREF] = { "symref", SOURCE_NONE, FIELD_STR, refname_atom_parser },
+	[ATOM_SYMREF] = { "symref", SOURCE_NONE, FIELD_STR, symref_atom_parser },
 	[ATOM_FLAG] = { "flag", SOURCE_NONE },
 	[ATOM_HEAD] = { "HEAD", SOURCE_NONE, FIELD_STR, head_atom_parser },
 	[ATOM_COLOR] = { "color", SOURCE_NONE, FIELD_STR, color_atom_parser },
@@ -770,8 +777,6 @@ static int parse_ref_filter_atom(struct ref_format *format,
 		return -1;
 	if (deref)
 		need_tagged = 1;
-	if (i == ATOM_SYMREF)
-		need_symref = 1;
 	return at;
 }
 
-- 
gitgitgadget

[PATCH 8/8] [GSOC] ref-filter: use switch/case instead of if/else

[ZheNing Hu via GitGitGadget]

From: ZheNing Hu <adlternative@gmail.com>

In the logic of ref-filter, if/else is used extensively
to check the atom type. This is because before we introduce
atom_type, the atom check is achieved through string comparison.

Using switch/case can allow the compiler to reduce unnecessary
branch checks and increase the readability of the code.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: ZheNing Hu <adlternative@gmail.com>
---
 ref-filter.c | 163 ++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 109 insertions(+), 54 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index f1c82e20e3d..c0b5d30ac57 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1110,9 +1110,11 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_
 		struct atom_value *v = &val[i];
 		if (!!deref != used_atom[i].deref)
 			continue;
-		if (atom_type == ATOM_OBJECTTYPE)
+		switch (atom_type) {
+		case ATOM_OBJECTTYPE : {
 			v->s = xstrdup(type_name(oi->type));
-		else if (atom_type == ATOM_OBJECTSIZE) {
+			break;
+		} case ATOM_OBJECTSIZE : {
 			if (used_atom[i].u.objectsize.option == O_SIZE_DISK) {
 				v->value = oi->disk_size;
 				v->s = xstrfmt_len(&v->s_size, "%"PRIuMAX, (uintmax_t)oi->disk_size);
@@ -1120,10 +1122,20 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_
 				v->value = oi->size;
 				v->s = xstrfmt_len(&v->s_size, "%"PRIuMAX , (uintmax_t)oi->size);
 			}
-		} else if (atom_type == ATOM_DELTABASE)
+			break;
+		}
+		case ATOM_DELTABASE: {
 			v->s = xstrdup(oid_to_hex(&oi->delta_base_oid));
-		else if (atom_type == ATOM_OBJECTNAME && deref) {
-			v->s = xstrdup(do_grab_oid("objectname", &oi->oid, &used_atom[i]));
+			break;
+		}
+		case ATOM_OBJECTNAME: {
+			if (deref)
+				v->s = xstrdup(do_grab_oid("objectname", &oi->oid, &used_atom[i]));
+			break;
+		}
+		default: {
+			break;
+		}
 		}
 	}
 }
@@ -1138,13 +1150,22 @@ static void grab_tag_values(struct atom_value *val, int deref, struct object *ob
 		enum atom_type atom_type = used_atom[i].atom_type;
 		struct atom_value *v = &val[i];
 		if (!!deref != used_atom[i].deref)
-			continue;
-		if (atom_type == ATOM_TAG)
+			break;
+		switch (atom_type) {
+		case ATOM_TAG:
 			v->s = xstrdup(tag->tag);
-		else if (atom_type == ATOM_TYPE && tag->tagged)
-			v->s = xstrdup(type_name(tag->tagged->type));
-		else if (atom_type == ATOM_OBJECT && tag->tagged)
-			v->s = xstrdup(oid_to_hex(&tag->tagged->oid));
+			break;
+		case ATOM_TYPE:
+			if (tag->tagged)
+				v->s = xstrdup(type_name(tag->tagged->type));
+			break;
+		case ATOM_OBJECT:
+			if (tag->tagged)
+				v->s = xstrdup(oid_to_hex(&tag->tagged->oid));
+			break;
+		default:
+			break;
+		}
 	}
 }
 
@@ -1159,15 +1180,17 @@ static void grab_commit_values(struct atom_value *val, int deref, struct object
 		struct atom_value *v = &val[i];
 		if (!!deref != used_atom[i].deref)
 			continue;
-		if (atom_type == ATOM_TREE) {
+		switch (atom_type) {
+		case ATOM_TREE: {
 			v->s = xstrdup(do_grab_oid("tree", get_commit_tree_oid(commit), &used_atom[i]));
-			continue;
+			break;
 		}
-		if (atom_type == ATOM_NUMPARENT) {
+		case ATOM_NUMPARENT: {
 			v->value = commit_list_count(commit->parents);
 			v->s = xstrfmt_len(&v->s_size, "%lu", (unsigned long)v->value);
+			break;
 		}
-		else if (atom_type == ATOM_PARENT) {
+		case ATOM_PARENT: {
 			struct commit_list *parents;
 			struct strbuf s = STRBUF_INIT;
 			for (parents = commit->parents; parents; parents = parents->next) {
@@ -1177,6 +1200,11 @@ static void grab_commit_values(struct atom_value *val, int deref, struct object
 				strbuf_addstr(&s, do_grab_oid("parent", oid, &used_atom[i]));
 			}
 			v->s = strbuf_detach(&s, NULL);
+			break;
+		}
+		default: {
+			break;
+		}
 		}
 	}
 }
@@ -1843,24 +1871,40 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 		v->handler = append_atom;
 		v->atom = atom;
 
-		if (atom_type == ATOM_REFNAME)
+		switch (atom_type) {
+		case ATOM_REFNAME: {
 			refname = get_refname(atom, ref);
-		else if (atom_type == ATOM_WORKTREEPATH) {
+			if (!deref)
+				v->s = xstrdup(refname);
+			else
+				v->s = xstrfmt_len(&v->s_size, "%s^{}", refname);
+			free((char *)refname);
+			break;
+		}
+		case ATOM_WORKTREEPATH: {
 			if (ref->kind == FILTER_REFS_BRANCHES)
 				v->s = get_worktree_path(atom, ref);
 			else
 				v->s = ref_filter_slopbuf;
-			continue;
+			break;
 		}
-		else if (atom_type == ATOM_SYMREF)
+		case ATOM_SYMREF: {
 			refname = get_symref(atom, ref);
-		else if (atom_type == ATOM_UPSTREAM) {
+			if (!deref)
+				v->s = xstrdup(refname);
+			else
+				v->s = xstrfmt_len(&v->s_size, "%s^{}", refname);
+			if (refname != ref_filter_slopbuf)
+				free((char *)refname);
+			break;
+		}
+		case ATOM_UPSTREAM: {
 			const char *branch_name;
 			/* only local branches may have an upstream */
 			if (!skip_prefix(ref->refname, "refs/heads/",
 					 &branch_name)) {
 				v->s = ref_filter_slopbuf;
-				continue;
+			break;
 			}
 			branch = branch_get(branch_name);
 
@@ -1869,13 +1913,17 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 				fill_remote_ref_details(atom, refname, branch, v);
 			else
 				v->s = ref_filter_slopbuf;
-			continue;
-		} else if (atom_type == ATOM_PUSH && atom->u.remote_ref.push) {
+			break;
+		}
+		case ATOM_PUSH: {
 			const char *branch_name;
+
+			if (!atom->u.remote_ref.push)
+				break;
 			v->s = ref_filter_slopbuf;
 			if (!skip_prefix(ref->refname, "refs/heads/",
 					 &branch_name))
-				continue;
+				break;
 			branch = branch_get(branch_name);
 
 			if (atom->u.remote_ref.push_remote)
@@ -1883,14 +1931,16 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 			else {
 				refname = branch_get_push(branch, NULL);
 				if (!refname)
-					continue;
+					break;
 			}
 			fill_remote_ref_details(atom, refname, branch, v);
-			continue;
-		} else if (atom_type == ATOM_COLOR) {
+			break;
+		}
+		case ATOM_COLOR: {
 			v->s = xstrdup(atom->u.color);
 			continue;
-		} else if (atom_type == ATOM_FLAG) {
+		}
+		case ATOM_FLAG: {
 			char buf[256], *cp = buf;
 			if (ref->flag & REF_ISSYMREF)
 				cp = copy_advance(cp, ",symref");
@@ -1902,54 +1952,59 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 				*cp = '\0';
 				v->s = xstrdup(buf + 1);
 			}
-			continue;
-		} else if (!deref && atom_type == ATOM_OBJECTNAME) {
-			   v->s = xstrdup(do_grab_oid("objectname", &ref->objectname, atom));
-			   continue;
-		} else if (atom_type == ATOM_HEAD) {
+			break;
+		}
+		case ATOM_OBJECTNAME: {
+			if (!deref)
+				v->s = xstrdup(do_grab_oid("objectname", &ref->objectname, atom));
+			break;
+		}
+		case ATOM_HEAD: {
 			if (atom->u.head && !strcmp(ref->refname, atom->u.head))
 				v->s = xstrdup("*");
 			else
 				v->s = xstrdup(" ");
-			continue;
-		} else if (atom_type == ATOM_ALIGN) {
+			break;
+		}
+		case ATOM_ALIGN: {
 			v->handler = align_atom_handler;
 			v->s = ref_filter_slopbuf;
-			continue;
-		} else if (atom_type == ATOM_END) {
+			break;
+		}
+		case ATOM_END: {
 			v->handler = end_atom_handler;
 			v->s = ref_filter_slopbuf;
-			continue;
-		} else if (atom_type == ATOM_IF) {
+			break;
+		}
+		case ATOM_IF: {
 			const char *s;
 			if (skip_prefix(name, "if:", &s))
 				v->s = xstrdup(s);
 			else
 				v->s = ref_filter_slopbuf;
 			v->handler = if_atom_handler;
-			continue;
-		} else if (atom_type == ATOM_THEN) {
+			break;
+		}
+		case ATOM_THEN: {
 			v->handler = then_atom_handler;
 			v->s = ref_filter_slopbuf;
-			continue;
-		} else if (atom_type == ATOM_ELSE) {
+			break;
+		}
+		case ATOM_ELSE: {
 			v->handler = else_atom_handler;
 			v->s = ref_filter_slopbuf;
-			continue;
-		} else if (atom_type == ATOM_REST) {
+			break;
+		}
+		case ATOM_REST: {
 			if (ref->rest)
 				v->s = xstrdup(ref->rest);
 			else
 				v->s = ref_filter_slopbuf;
-			continue;
-		} else
-			continue;
-
-		if (!deref)
-			v->s = xstrdup(refname);
-		else
-			v->s = xstrfmt_len(&v->s_size, "%s^{}", refname);
-		free((char *)refname);
+			break;
+		}
+		default:
+			break;
+		}
 	}
 
 	for (i = 0; i < used_atom_cnt; i++) {
-- 
gitgitgadget