On 9/19/2019 5:47 PM, SZEDER Gábor wrote:
> In a later patch in this series we'll want to do this in two places.
>
> Signed-off-by: SZEDER Gábor <szeder....@gmail.com>
> ---
> builtin/name-rev.c | 40 +++++++++++++++++++++++++++-------------
> 1 file changed, 27 insertions(+), 13 deletions(-)
>
> diff --git a/builtin/name-rev.c b/builtin/name-rev.c
> index dec2228cc7..cb8ac2fa64 100644
> --- a/builtin/name-rev.c
> +++ b/builtin/name-rev.c
> @@ -75,12 +75,36 @@ static int is_better_name(struct rev_name *name,
> return 0;
> }
>
> +static struct rev_name *create_or_update_name(struct commit *commit,
> + const char *tip_name,
> + timestamp_t taggerdate,
> + int generation, int distance,
> + int from_tag)
> +{
> + struct rev_name *name = get_commit_rev_name(commit);
> +
> + 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;
> +}
> +
> static void name_rev(struct commit *commit,
> const char *tip_name, timestamp_t taggerdate,
> int generation, int distance, int from_tag,
> int deref)
> {
> - struct rev_name *name = get_commit_rev_name(commit);
A perhaps small benefit: we delay this call until after some
other checks happen. It's just looking up data in a cache, but
it may help a little.
> struct commit_list *parents;
> int parent_number = 1;
> char *to_free = NULL;
> @@ -97,18 +121,8 @@ static void name_rev(struct commit *commit,
> die("generation: %d, but deref?", generation);
> }
>
> - 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;
> - } else {
> + if (!create_or_update_name(commit, tip_name, taggerdate, generation,
> + distance, from_tag)) {
Otherwise this method extraction looks correct.
Thanks,
-Stolee
