On Sat, Jul 30, 2016 at 12:11:11PM -0700, Josh Triplett wrote:

> +enum from {
> +     FROM_AUTHOR,
> +     FROM_USER,
> +     FROM_VALUE,
> +};
> +
> +static void set_from(enum from type, const char *value)
> +{
> +     free(from);
> +     switch (type) {
> +     case FROM_AUTHOR:
> +             from = NULL;
> +             break;
> +     case FROM_USER:
> +             from = xstrdup(git_committer_info(IDENT_NO_DATE));
> +             break;
> +     case FROM_VALUE:
> +             from = xstrdup(value);
> +             break;
> +     }
> +}

Thanks for looking into reducing the duplication. TBH, I am not sure it
is really an improvement, just because of the amount of boilerplate (and
this function interface is kind of weird, because of the rules for when
"value" should or should not be NULL).

I guess another way to do it would be:

  #define FROM_AUTO_IDENT ((const char *)(intptr_t)1))
  void set_from(const char *value)
  {
        if (value == FROM_AUTO_IDENT)
                value = git_committer_info(IDENT_NO_DATE);
        free(from);
        from = xstrdup_or_null(value);
  }

but I think the effort to polish further here is outweighing the
magnitude of the patch itself. So I offer that as "how I would have done
it" in case you like it, but again, I am fine with either this version
or the previous.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to