This code was moved straight out of name_rev(). As such, we inherited
the "goto" to jump from an if into an else-if. We also inherited the
fact that "nothing to do -- return NULL" is handled last.
Rewrite the function to first handle the "nothing to do" case. Then we
can handle the conditional allocation early before going on to populate
the struct. No need for goto-ing.
Signed-off-by: Martin Ågren <martin.agren@xxxxxxxxx>
---
Hi SZEDER,
For the record, --color-moved confirms that your patch is a move and
the conversion around it looks good to me. I was a bit puzzled by what
the moved code actually wanted to *do* and came up with this rewrite.
I guess it's subjective which of these ways of writing this function is
"better", but I found this rewrite helped in understanding this
function. Feel free to pick up in a reroll, squash or ignore as you see
fit. This is based on top of your whole series (your e5d77042f), but
could perhaps go immediately after your patch 07/15.
It seems there was some discussion around leaks and leak-plugs. That
would conflict/interact with this. Instead of placing a call to free()
just before the label so we can more or less goto around it, the middle
section of this rewritten function would turn from "if no name,
allocate one" to "if we have a name, free stuff, else allocate one".
Again, it's subjective which way is "better", so trust your judgment.
Martin
builtin/name-rev.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 6969af76c4..03a5f0b189 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -83,21 +83,21 @@ static struct rev_name *create_or_update_name(struct commit *commit,
{
struct rev_name *name = get_commit_rev_name(commit);
+ if (name && !is_better_name(name, taggerdate, distance, from_tag))
+ return NULL;
+
if (name == NULL) {
name = xmalloc(sizeof(*name));
set_commit_rev_name(commit, name);
- goto copy_data;
- } else if (is_better_name(name, taggerdate, distance, from_tag)) {
-copy_data:
- name->tip_name = tip_name;
- name->taggerdate = taggerdate;
- name->generation = generation;
- name->distance = distance;
- name->from_tag = from_tag;
-
- return name;
- } else
- return NULL;
+ }
+
+ name->tip_name = tip_name;
+ name->taggerdate = taggerdate;
+ name->generation = generation;
+ name->distance = distance;
+ name->from_tag = from_tag;
+
+ return name;
}
static void name_rev(struct commit *start_commit,
--
2.23.0
