Re: [PATCH v2] gpg-interface: add function for converting trust level to string

[Eric Sunshine <sunshine@xxxxxxxxxxxxxx>]

On Sat, Jul 9, 2022 at 4:52 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
> > On Fri, Jul 8, 2022 at 7:28 AM Jaydeep Das via GitGitGadget
> >> + * Returns corresponding string in lowercase for a given member of
> >> + * enum signature_trust_level. For example, `TRUST_ULTIMATE` will
> >> + * return "ultimate".
> >> +char *gpg_trust_level_to_str(enum signature_trust_level level);
> >
> > It would be a good idea to update the function documentation to
> > mention that the caller is responsible for freeing the returned
> > string.
>
> Given that there are small and fixed number of trust level strings,
> I actually think that it would be more reasonable to return a static
> string to the caller, something along the lines of the attached, so
> that callers do not have to worry about freeing it.

I also am not a fan of making the caller free the result, and thought
of mentioning it but didn't know if the approach implemented by this
patch was suggested by an earlier reviewer.

> Perhaps along the lines of ...
>
> +static struct sigcheck_gpg_trust_level {
>         const char *key;
>         enum signature_trust_level value;
> +       const char *downcased;
>  } sigcheck_gpg_trust_level[] = {
>
> +const char *gpg_trust_level_to_string(enum signature_trust_level level)
> +{
> +       struct sigcheck_gpg_trust_level *trust;
> +
> +       if (level < 0 || ARRAY_SIZE(sigcheck_gpg_trust_level) <= level)
> +               BUG("invalid trust_level requested: %d", level);
> +
> +       trust = &sigcheck_gpg_trust_level[level];
> +       if (trust->value != level)
> +               BUG("sigcheck_gpg_trust_level[] unsorted");
> +
> +       if (!trust->downcased)
> +               trust->downcased = xstrdup_tolower(trust->key);
> +       return trust->downcased;
> +}

Given the small, fixed number of trust levels, and if the list is
unlikely to change much in the future, I might suggest simply
initializing the fields at compile-time rather than on-demand at
run-time:

    static struct {
        const char *key;
        const char *display_key;
        enum signature_trust_level value;
    } sigcheck_gpg_trust_level[] = {
        { "UNDEFINED", "undefined", TRUST_UNDEFINED },
        { "NEVER", "never", TRUST_NEVER },
        { "MARGINAL", "marginal", TRUST_MARGINAL },
        { "FULLY", "fully", TRUST_FULLY },
        { "ULTIMATE", "ultimate", TRUST_ULTIMATE },
    };