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

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

[Jaydeep Das via GitGitGadget]

From: Jaydeep P Das <jaydeepjd.8914@gmail.com>

Add new helper function `gpg_trust_level_to_str()` which will
convert a given member of `enum signature_trust_level` to its
corresponding string. For example, `TRUST_ULTIMATE`
will yield the string "ULTIMATE".

This will abstract out some code in `pretty.c` relating to gpg
signature trust levels.

Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Jaydeep Das <jaydeepjd.8914@gmail.com>
---
    gpg-interface: add function for converting trust level to string
    
    Add new helper function gpg_trust_level_to_str() which will convert a
    given member of enum signature_trust_level to its corresponding string.
    For example, TRUST_ULTIMATE will yield the string "ULTIMATE".
    
    This will abstract out some code in pretty.c relating to gpg signature
    trust levels.
    
    Mentored-by: Christian Couder chriscool@tuxfamily.org Mentored-by:
    Hariom Verma hariom18599@gmail.com Signed-off-by: Jaydeep Das
    jaydeepjd.8914@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1281%2FJDeepD%2Fgpg-wrap-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1281/JDeepD/gpg-wrap-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1281

 gpg-interface.c |  7 +++++++
 gpg-interface.h |  8 ++++++++
 pretty.c        | 23 ++++++-----------------
 3 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/gpg-interface.c b/gpg-interface.c
index 947b58ad4da..fe6e5ce5127 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -165,6 +165,7 @@ static struct {
 	{ 0, "TRUST_", GPG_STATUS_TRUST_LEVEL },
 };
 
+/* Keep the order same as enum signature_trust_level */
 static struct {
 	const char *key;
 	enum signature_trust_level value;
@@ -905,6 +906,12 @@ const char *get_signing_key(void)
 	return git_committer_info(IDENT_STRICT | IDENT_NO_DATE);
 }
 
+const char *gpg_trust_level_to_str(enum signature_trust_level level){
+	if (level < TRUST_UNDEFINED || level > TRUST_ULTIMATE)
+		return NULL;
+	return sigcheck_gpg_trust_level[level].key;
+}
+
 int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key)
 {
 	return use_format->sign_buffer(buffer, signature, signing_key);
diff --git a/gpg-interface.h b/gpg-interface.h
index b30cbdcd3da..48f7edd916b 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -71,6 +71,14 @@ size_t parse_signed_buffer(const char *buf, size_t size);
 int sign_buffer(struct strbuf *buffer, struct strbuf *signature,
 		const char *signing_key);
 
+
+/*
+ * Returns corresponding string for a given member of
+ * enum signature_trust_level. For example, `TRUST_ULTIMATE` will
+ * return "ULTIMATE".
+ */
+const char *gpg_trust_level_to_str(enum signature_trust_level level);
+
 int git_gpg_config(const char *, const char *, void *);
 void set_signing_key(const char *);
 const char *get_signing_key(void);
diff --git a/pretty.c b/pretty.c
index ee6114e3f0a..f617dd601ac 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1347,7 +1347,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 	const struct commit *commit = c->commit;
 	const char *msg = c->message;
 	struct commit_list *p;
-	const char *arg, *eol;
+	const char *arg, *eol, *sig_str;
 	size_t res;
 	char **slot;
 
@@ -1575,22 +1575,11 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 				strbuf_addstr(sb, c->signature_check.primary_key_fingerprint);
 			break;
 		case 'T':
-			switch (c->signature_check.trust_level) {
-			case TRUST_UNDEFINED:
-				strbuf_addstr(sb, "undefined");
-				break;
-			case TRUST_NEVER:
-				strbuf_addstr(sb, "never");
-				break;
-			case TRUST_MARGINAL:
-				strbuf_addstr(sb, "marginal");
-				break;
-			case TRUST_FULLY:
-				strbuf_addstr(sb, "fully");
-				break;
-			case TRUST_ULTIMATE:
-				strbuf_addstr(sb, "ultimate");
-				break;
+			sig_str = gpg_trust_level_to_str(c->signature_check.trust_level);
+			if (sig_str){
+				const char *sig_str_lower = xstrdup_tolower(sig_str);
+				strbuf_addstr(sb, sig_str_lower);
+				free((char *)sig_str_lower);
 			}
 			break;
 		default:

base-commit: 30cc8d0f147546d4dd77bf497f4dec51e7265bd8
-- 
gitgitgadget

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

[Junio C Hamano]

"Jaydeep Das via GitGitGadget" <gitgitgadget@gmail.com> writes:

> diff --git a/gpg-interface.c b/gpg-interface.c
> index 947b58ad4da..fe6e5ce5127 100644
> --- a/gpg-interface.c
> +++ b/gpg-interface.c
> @@ -165,6 +165,7 @@ static struct {
>  	{ 0, "TRUST_", GPG_STATUS_TRUST_LEVEL },
>  };
>  
> +/* Keep the order same as enum signature_trust_level */
>  static struct {
>  	const char *key;
>  	enum signature_trust_level value;
> @@ -905,6 +906,12 @@ const char *get_signing_key(void)
>  	return git_committer_info(IDENT_STRICT | IDENT_NO_DATE);
>  }
>  
> +const char *gpg_trust_level_to_str(enum signature_trust_level level){
> +	if (level < TRUST_UNDEFINED || level > TRUST_ULTIMATE)
> +		return NULL;
> +	return sigcheck_gpg_trust_level[level].key;
> +}
> +
>  int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key)
>  {
>  	return use_format->sign_buffer(buffer, signature, signing_key);
> diff --git a/gpg-interface.h b/gpg-interface.h
> index b30cbdcd3da..48f7edd916b 100644
> --- a/gpg-interface.h
> +++ b/gpg-interface.h
> @@ -71,6 +71,14 @@ size_t parse_signed_buffer(const char *buf, size_t size);
>  int sign_buffer(struct strbuf *buffer, struct strbuf *signature,
>  		const char *signing_key);
>  
> +
> +/*
> + * Returns corresponding string for a given member of
> + * enum signature_trust_level. For example, `TRUST_ULTIMATE` will
> + * return "ULTIMATE".
> + */
> +const char *gpg_trust_level_to_str(enum signature_trust_level level);
> +			sig_str = gpg_trust_level_to_str(c->signature_check.trust_level);
> +			if (sig_str){

Missing SP before open-brace.

> +				const char *sig_str_lower = xstrdup_tolower(sig_str);
> +				strbuf_addstr(sb, sig_str_lower);
> +				free((char *)sig_str_lower);

Unnecessary const plus casting-away of it.  You are getting a copy
to work with, so there is no reason to declare sig_str_lower to be
"const".  This downcasing should be done in gpg_trust_level_to_str()
function, shouldn't it?  After all, the "str" version of the trust
level existing end-users are familiar with are the strings you
removed from pretty.c that are all lowercase.

Thanks.

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

[Jaydeep Das via GitGitGadget]

From: Jaydeep Das <jaydeepjd.8914@gmail.com>

Add new helper function `gpg_trust_level_to_str()` which will
convert a given member of `enum signature_trust_level` to its
corresponding string(in lowercase). For example, `TRUST_ULTIMATE`
will yield the string "ultimate".

This will abstract out some code in `pretty.c` relating to gpg
signature trust levels.

Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Jaydeep Das <jaydeepjd.8914@gmail.com>
---
    gpg-interface: add function for converting trust level to string
    
    Add new helper function gpg_trust_level_to_str() which will convert a
    given member of enum signature_trust_level to its corresponding string
    in lowercase. For example, TRUST_ULTIMATE will yield the string
    "ultimate".
    
    This will abstract out some code in pretty.c relating to gpg signature
    trust levels.
    
    Changes since v1:
    
     * gpg_trust_level_to_str() now returns the string in lowercase
    
    Mentored-by: Christian Couder chriscool@tuxfamily.org Mentored-by:
    Hariom Verma hariom18599@gmail.com Signed-off-by: Jaydeep Das
    jaydeepjd.8914@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1281%2FJDeepD%2Fgpg-wrap-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1281/JDeepD/gpg-wrap-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1281

Range-diff vs v1:

 1:  fbbad9cc26a ! 1:  640decc2afe gpg-interface: add function for converting trust level to string
     @@
       ## Metadata ##
     -Author: Jaydeep P Das <jaydeepjd.8914@gmail.com>
     +Author: Jaydeep Das <jaydeepjd.8914@gmail.com>
      
       ## Commit message ##
          gpg-interface: add function for converting trust level to string
      
          Add new helper function `gpg_trust_level_to_str()` which will
          convert a given member of `enum signature_trust_level` to its
     -    corresponding string. For example, `TRUST_ULTIMATE`
     -    will yield the string "ULTIMATE".
     +    corresponding string(in lowercase). For example, `TRUST_ULTIMATE`
     +    will yield the string "ultimate".
      
          This will abstract out some code in `pretty.c` relating to gpg
          signature trust levels.
     @@ gpg-interface.c: const char *get_signing_key(void)
       	return git_committer_info(IDENT_STRICT | IDENT_NO_DATE);
       }
       
     -+const char *gpg_trust_level_to_str(enum signature_trust_level level){
     ++char *gpg_trust_level_to_str(enum signature_trust_level level){
      +	if (level < TRUST_UNDEFINED || level > TRUST_ULTIMATE)
      +		return NULL;
     -+	return sigcheck_gpg_trust_level[level].key;
     ++	return xstrdup_tolower(sigcheck_gpg_trust_level[level].key);
      +}
      +
       int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key)
     @@ gpg-interface.h: size_t parse_signed_buffer(const char *buf, size_t size);
       
      +
      +/*
     -+ * Returns corresponding string for a given member of
     ++ * Returns corresponding string in lowercase for a given member of
      + * enum signature_trust_level. For example, `TRUST_ULTIMATE` will
     -+ * return "ULTIMATE".
     ++ * return "ultimate".
      + */
     -+const char *gpg_trust_level_to_str(enum signature_trust_level level);
     ++char *gpg_trust_level_to_str(enum signature_trust_level level);
      +
       int git_gpg_config(const char *, const char *, void *);
       void set_signing_key(const char *);
     @@ gpg-interface.h: size_t parse_signed_buffer(const char *buf, size_t size);
      
       ## pretty.c ##
      @@ pretty.c: static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
     - 	const struct commit *commit = c->commit;
       	const char *msg = c->message;
       	struct commit_list *p;
     --	const char *arg, *eol;
     -+	const char *arg, *eol, *sig_str;
     + 	const char *arg, *eol;
     ++	char *sig_str;
       	size_t res;
       	char **slot;
       
     @@ pretty.c: static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
      -			case TRUST_ULTIMATE:
      -				strbuf_addstr(sb, "ultimate");
      -				break;
     +-			}
      +			sig_str = gpg_trust_level_to_str(c->signature_check.trust_level);
     -+			if (sig_str){
     -+				const char *sig_str_lower = xstrdup_tolower(sig_str);
     -+				strbuf_addstr(sb, sig_str_lower);
     -+				free((char *)sig_str_lower);
     - 			}
     ++			if (sig_str)
     ++				strbuf_addstr(sb, sig_str);
     ++			free(sig_str);
       			break;
       		default:
     + 			return 0;


 gpg-interface.c |  7 +++++++
 gpg-interface.h |  8 ++++++++
 pretty.c        | 22 +++++-----------------
 3 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/gpg-interface.c b/gpg-interface.c
index 947b58ad4da..4ef660a09fc 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -165,6 +165,7 @@ static struct {
 	{ 0, "TRUST_", GPG_STATUS_TRUST_LEVEL },
 };
 
+/* Keep the order same as enum signature_trust_level */
 static struct {
 	const char *key;
 	enum signature_trust_level value;
@@ -905,6 +906,12 @@ const char *get_signing_key(void)
 	return git_committer_info(IDENT_STRICT | IDENT_NO_DATE);
 }
 
+char *gpg_trust_level_to_str(enum signature_trust_level level){
+	if (level < TRUST_UNDEFINED || level > TRUST_ULTIMATE)
+		return NULL;
+	return xstrdup_tolower(sigcheck_gpg_trust_level[level].key);
+}
+
 int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key)
 {
 	return use_format->sign_buffer(buffer, signature, signing_key);
diff --git a/gpg-interface.h b/gpg-interface.h
index b30cbdcd3da..ce2db6f3780 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -71,6 +71,14 @@ size_t parse_signed_buffer(const char *buf, size_t size);
 int sign_buffer(struct strbuf *buffer, struct strbuf *signature,
 		const char *signing_key);
 
+
+/*
+ * 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);
+
 int git_gpg_config(const char *, const char *, void *);
 void set_signing_key(const char *);
 const char *get_signing_key(void);
diff --git a/pretty.c b/pretty.c
index ee6114e3f0a..5ee03d6fe09 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1348,6 +1348,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 	const char *msg = c->message;
 	struct commit_list *p;
 	const char *arg, *eol;
+	char *sig_str;
 	size_t res;
 	char **slot;
 
@@ -1575,23 +1576,10 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 				strbuf_addstr(sb, c->signature_check.primary_key_fingerprint);
 			break;
 		case 'T':
-			switch (c->signature_check.trust_level) {
-			case TRUST_UNDEFINED:
-				strbuf_addstr(sb, "undefined");
-				break;
-			case TRUST_NEVER:
-				strbuf_addstr(sb, "never");
-				break;
-			case TRUST_MARGINAL:
-				strbuf_addstr(sb, "marginal");
-				break;
-			case TRUST_FULLY:
-				strbuf_addstr(sb, "fully");
-				break;
-			case TRUST_ULTIMATE:
-				strbuf_addstr(sb, "ultimate");
-				break;
-			}
+			sig_str = gpg_trust_level_to_str(c->signature_check.trust_level);
+			if (sig_str)
+				strbuf_addstr(sb, sig_str);
+			free(sig_str);
 			break;
 		default:
 			return 0;

base-commit: 30cc8d0f147546d4dd77bf497f4dec51e7265bd8
-- 
gitgitgadget

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

[Eric Sunshine]

On Fri, Jul 8, 2022 at 7:28 AM Jaydeep Das via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> Add new helper function `gpg_trust_level_to_str()` which will
> convert a given member of `enum signature_trust_level` to its
> corresponding string(in lowercase). For example, `TRUST_ULTIMATE`

s/g(/g (/

> will yield the string "ultimate".
>
> This will abstract out some code in `pretty.c` relating to gpg
> signature trust levels.
>
> Signed-off-by: Jaydeep Das <jaydeepjd.8914@gmail.com>
> ---
> diff --git a/gpg-interface.h b/gpg-interface.h
> @@ -71,6 +71,14 @@ size_t parse_signed_buffer(const char *buf, size_t size);
> +/*
> + * 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.

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

[jaydeepjd.8914]

On 7/9/22 6:28 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Fri, Jul 8, 2022 at 7:28 AM Jaydeep Das via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> > Signed-off-by: Jaydeep Das <jaydeepjd.8914@gmail.com>
> > ---
> > diff --git a/gpg-interface.h b/gpg-interface.h
> > @@ -71,6 +71,14 @@ size_t parse_signed_buffer(const char *buf, size_t size);
> > +/*
> > + * 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.

Yeah. Will do in next version.

Thanks,
Jaydeep.

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

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Fri, Jul 8, 2022 at 7:28 AM Jaydeep Das via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>> Add new helper function `gpg_trust_level_to_str()` which will
>> convert a given member of `enum signature_trust_level` to its
>> corresponding string(in lowercase). For example, `TRUST_ULTIMATE`
>
> s/g(/g (/
>
>> will yield the string "ultimate".
>>
>> This will abstract out some code in `pretty.c` relating to gpg
>> signature trust levels.
>>
>> Signed-off-by: Jaydeep Das <jaydeepjd.8914@gmail.com>
>> ---
>> diff --git a/gpg-interface.h b/gpg-interface.h
>> @@ -71,6 +71,14 @@ size_t parse_signed_buffer(const char *buf, size_t size);
>> +/*
>> + * 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.

Perhaps along the lines of ...

 gpg-interface.c | 19 ++++++++++++++++++-
 gpg-interface.h |  2 ++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git c/gpg-interface.c w/gpg-interface.c
index 947b58ad4d..4a5b9d0f3a 100644
--- c/gpg-interface.c
+++ w/gpg-interface.c
@@ -165,9 +165,10 @@ static struct {
 	{ 0, "TRUST_", GPG_STATUS_TRUST_LEVEL },
 };
 
-static struct {
+static struct sigcheck_gpg_trust_level {
 	const char *key;
 	enum signature_trust_level value;
+	const char *downcased;
 } sigcheck_gpg_trust_level[] = {
 	{ "UNDEFINED", TRUST_UNDEFINED },
 	{ "NEVER", TRUST_NEVER },
@@ -176,6 +177,22 @@ static struct {
 	{ "ULTIMATE", TRUST_ULTIMATE },
 };
 
+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;
+}
+
 static void replace_cstring(char **field, const char *line, const char *next)
 {
 	free(*field);
diff --git c/gpg-interface.h w/gpg-interface.h
index b30cbdcd3d..2dffcb836d 100644
--- c/gpg-interface.h
+++ w/gpg-interface.h
@@ -85,4 +85,6 @@ int check_signature(struct signature_check *sigc,
 void print_signature_buffer(const struct signature_check *sigc,
 			    unsigned flags);
 
+const char *gpg_trust_level_to_string(enum signature_trust_level);
+
 #endif

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

[Eric Sunshine]

On Sat, Jul 9, 2022 at 4:52 PM Junio C Hamano <gitster@pobox.com> wrote:
> Eric Sunshine <sunshine@sunshineco.com> 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 },
    };

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

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> 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 },
>     };

Yup, that is even better.  I wonder if we can upcase in C
preprocessor macro?  It would be wonderful if we can do so,
but for just 5 entries, we can type each token three times
just fine.

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

[Eric Sunshine]

On Sun, Jul 10, 2022 at 1:48 AM Junio C Hamano <gitster@pobox.com> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> > 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 },
> >     };
>
> Yup, that is even better.  I wonder if we can upcase in C
> preprocessor macro?  It would be wonderful if we can do so,
> but for just 5 entries, we can type each token three times
> just fine.

No standardized way to upcase via the C preprocessor, as far as I
know. At any rate, I suspect such a macro would be uglier and harder
to reason about than the code above, which is dead-simple to
understand.

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

[Jaydeep Das]

On 7/10/22 11:14, Eric Sunshine wrote:
  
> 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.
> 
> 
> 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 },
>      };

Will do in next patch.

Thanks,
Jaydeep

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

[Jaydeep Das via GitGitGadget]

From: Jaydeep Das <jaydeepjd.8914@gmail.com>

Add new helper function `gpg_trust_level_to_str()` which will
convert a given member of `enum signature_trust_level` to its
corresponding string (in lowercase). For example, `TRUST_ULTIMATE`
will yield the string "ultimate".

This will abstract out some code in `pretty.c` relating to gpg
signature trust levels.

Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Jaydeep Das <jaydeepjd.8914@gmail.com>
---
    gpg-interface: add function for converting trust level to string
    
    Add new helper function gpg_trust_level_to_str() which will convert a
    given member of enum signature_trust_level to its corresponding string
    in lowercase. For example, TRUST_ULTIMATE will yield the string
    "ultimate".
    
    This will abstract out some code in pretty.c relating to gpg signature
    trust levels.
    
    Changes since v1:
    
     * gpg_trust_level_to_str() now returns the string in lowercase.
    
    Changes since v2:
    
     * Updated docs.
    
    Mentored-by: Christian Couder chriscool@tuxfamily.org Mentored-by:
    Hariom Verma hariom18599@gmail.com Signed-off-by: Jaydeep Das
    jaydeepjd.8914@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1281%2FJDeepD%2Fgpg-wrap-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1281/JDeepD/gpg-wrap-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1281

Range-diff vs v2:

 1:  640decc2afe ! 1:  933d6caa916 gpg-interface: add function for converting trust level to string
     @@ Commit message
      
          Add new helper function `gpg_trust_level_to_str()` which will
          convert a given member of `enum signature_trust_level` to its
     -    corresponding string(in lowercase). For example, `TRUST_ULTIMATE`
     +    corresponding string (in lowercase). For example, `TRUST_ULTIMATE`
          will yield the string "ultimate".
      
          This will abstract out some code in `pretty.c` relating to gpg
     @@ gpg-interface.h: size_t parse_signed_buffer(const char *buf, size_t size);
      +/*
      + * Returns corresponding string in lowercase for a given member of
      + * enum signature_trust_level. For example, `TRUST_ULTIMATE` will
     -+ * return "ultimate".
     ++ * return "ultimate". Since it uses xstrdup_tolower(), which uses
     ++ * xmallocz(), the caller has to free up the memory for returned string
     ++ * after usage.
      + */
      +char *gpg_trust_level_to_str(enum signature_trust_level level);
      +


 gpg-interface.c |  7 +++++++
 gpg-interface.h | 10 ++++++++++
 pretty.c        | 22 +++++-----------------
 3 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/gpg-interface.c b/gpg-interface.c
index 947b58ad4da..4ef660a09fc 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -165,6 +165,7 @@ static struct {
 	{ 0, "TRUST_", GPG_STATUS_TRUST_LEVEL },
 };
 
+/* Keep the order same as enum signature_trust_level */
 static struct {
 	const char *key;
 	enum signature_trust_level value;
@@ -905,6 +906,12 @@ const char *get_signing_key(void)
 	return git_committer_info(IDENT_STRICT | IDENT_NO_DATE);
 }
 
+char *gpg_trust_level_to_str(enum signature_trust_level level){
+	if (level < TRUST_UNDEFINED || level > TRUST_ULTIMATE)
+		return NULL;
+	return xstrdup_tolower(sigcheck_gpg_trust_level[level].key);
+}
+
 int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key)
 {
 	return use_format->sign_buffer(buffer, signature, signing_key);
diff --git a/gpg-interface.h b/gpg-interface.h
index b30cbdcd3da..eda8b32015c 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -71,6 +71,16 @@ size_t parse_signed_buffer(const char *buf, size_t size);
 int sign_buffer(struct strbuf *buffer, struct strbuf *signature,
 		const char *signing_key);
 
+
+/*
+ * Returns corresponding string in lowercase for a given member of
+ * enum signature_trust_level. For example, `TRUST_ULTIMATE` will
+ * return "ultimate". Since it uses xstrdup_tolower(), which uses
+ * xmallocz(), the caller has to free up the memory for returned string
+ * after usage.
+ */
+char *gpg_trust_level_to_str(enum signature_trust_level level);
+
 int git_gpg_config(const char *, const char *, void *);
 void set_signing_key(const char *);
 const char *get_signing_key(void);
diff --git a/pretty.c b/pretty.c
index ee6114e3f0a..5ee03d6fe09 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1348,6 +1348,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 	const char *msg = c->message;
 	struct commit_list *p;
 	const char *arg, *eol;
+	char *sig_str;
 	size_t res;
 	char **slot;
 
@@ -1575,23 +1576,10 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 				strbuf_addstr(sb, c->signature_check.primary_key_fingerprint);
 			break;
 		case 'T':
-			switch (c->signature_check.trust_level) {
-			case TRUST_UNDEFINED:
-				strbuf_addstr(sb, "undefined");
-				break;
-			case TRUST_NEVER:
-				strbuf_addstr(sb, "never");
-				break;
-			case TRUST_MARGINAL:
-				strbuf_addstr(sb, "marginal");
-				break;
-			case TRUST_FULLY:
-				strbuf_addstr(sb, "fully");
-				break;
-			case TRUST_ULTIMATE:
-				strbuf_addstr(sb, "ultimate");
-				break;
-			}
+			sig_str = gpg_trust_level_to_str(c->signature_check.trust_level);
+			if (sig_str)
+				strbuf_addstr(sb, sig_str);
+			free(sig_str);
 			break;
 		default:
 			return 0;

base-commit: 30cc8d0f147546d4dd77bf497f4dec51e7265bd8
-- 
gitgitgadget

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

[Jaydeep Das via GitGitGadget]

From: Jaydeep Das <jaydeepjd.8914@gmail.com>

Add new helper function `gpg_trust_level_to_str()` which will
convert a given member of `enum signature_trust_level` to its
corresponding string (in lowercase). For example, `TRUST_ULTIMATE`
will yield the string "ultimate".

This will abstract out some code in `pretty.c` relating to gpg
signature trust levels.

Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Jaydeep Das <jaydeepjd.8914@gmail.com>
---
    gpg-interface: add function for converting trust level to string
    
    Add new helper function gpg_trust_level_to_str() which will convert a
    given member of enum signature_trust_level to its corresponding string
    in lowercase. For example, TRUST_ULTIMATE will yield the string
    "ultimate".
    
    This will abstract out some code in pretty.c relating to gpg signature
    trust levels.
    
    Changes since v1:
    
     * gpg_trust_level_to_str() now returns the string in lowercase.
    
    Changes since v2:
    
     * Updated docs.
    
    Changes since v3:
    
     * gpg_trust_level_to_str() now returns a static string. So the caller
       does not have to worry about free()ing it.
     * Updated pretty.c and docs accordingly.
    
    Mentored-by: Christian Couder chriscool@tuxfamily.org Mentored-by:
    Hariom Verma hariom18599@gmail.com Signed-off-by: Jaydeep Das
    jaydeepjd.8914@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1281%2FJDeepD%2Fgpg-wrap-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1281/JDeepD/gpg-wrap-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1281

Range-diff vs v3:

 1:  933d6caa916 ! 1:  7692b83821d gpg-interface: add function for converting trust level to string
     @@ gpg-interface.c: static struct {
       	{ 0, "TRUST_", GPG_STATUS_TRUST_LEVEL },
       };
       
     +-static struct {
      +/* Keep the order same as enum signature_trust_level */
     - static struct {
     ++static struct sigcheck_gpg_trust_level {
       	const char *key;
     ++	const char *display_key;
       	enum signature_trust_level value;
     + } sigcheck_gpg_trust_level[] = {
     +-	{ "UNDEFINED", TRUST_UNDEFINED },
     +-	{ "NEVER", TRUST_NEVER },
     +-	{ "MARGINAL", TRUST_MARGINAL },
     +-	{ "FULLY", TRUST_FULLY },
     +-	{ "ULTIMATE", TRUST_ULTIMATE },
     ++	{ "UNDEFINED", "undefined", TRUST_UNDEFINED },
     ++	{ "NEVER", "never", TRUST_NEVER },
     ++	{ "MARGINAL", "marginal", TRUST_MARGINAL },
     ++	{ "FULLY", "fully", TRUST_FULLY },
     ++	{ "ULTIMATE", "ultimate", TRUST_ULTIMATE },
     + };
     + 
     + static void replace_cstring(char **field, const char *line, const char *next)
      @@ gpg-interface.c: const char *get_signing_key(void)
       	return git_committer_info(IDENT_STRICT | IDENT_NO_DATE);
       }
       
     -+char *gpg_trust_level_to_str(enum signature_trust_level level){
     -+	if (level < TRUST_UNDEFINED || level > TRUST_ULTIMATE)
     -+		return NULL;
     -+	return xstrdup_tolower(sigcheck_gpg_trust_level[level].key);
     ++const char *gpg_trust_level_to_str(enum signature_trust_level level)
     ++{
     ++	struct sigcheck_gpg_trust_level *trust;
     ++
     ++	if (level < 0 || level >= ARRAY_SIZE(sigcheck_gpg_trust_level))
     ++		BUG("invalid trust level requested %d", level);
     ++
     ++	trust = &sigcheck_gpg_trust_level[level];
     ++	if (trust->value != level)
     ++		BUG("sigcheck_gpg_trust_level[] unsorted");
     ++
     ++	return sigcheck_gpg_trust_level[level].display_key;
      +}
      +
       int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key)
     @@ gpg-interface.h: size_t parse_signed_buffer(const char *buf, size_t size);
      +/*
      + * Returns corresponding string in lowercase for a given member of
      + * enum signature_trust_level. For example, `TRUST_ULTIMATE` will
     -+ * return "ultimate". Since it uses xstrdup_tolower(), which uses
     -+ * xmallocz(), the caller has to free up the memory for returned string
     -+ * after usage.
     ++ * return "ultimate".
      + */
     -+char *gpg_trust_level_to_str(enum signature_trust_level level);
     ++const char *gpg_trust_level_to_str(enum signature_trust_level level);
      +
       int git_gpg_config(const char *, const char *, void *);
       void set_signing_key(const char *);
       const char *get_signing_key(void);
      
       ## pretty.c ##
     -@@ pretty.c: static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
     - 	const char *msg = c->message;
     - 	struct commit_list *p;
     - 	const char *arg, *eol;
     -+	char *sig_str;
     - 	size_t res;
     - 	char **slot;
     - 
      @@ pretty.c: static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
       				strbuf_addstr(sb, c->signature_check.primary_key_fingerprint);
       			break;
     @@ pretty.c: static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
      -				strbuf_addstr(sb, "ultimate");
      -				break;
      -			}
     -+			sig_str = gpg_trust_level_to_str(c->signature_check.trust_level);
     -+			if (sig_str)
     -+				strbuf_addstr(sb, sig_str);
     -+			free(sig_str);
     ++			strbuf_addstr(sb, gpg_trust_level_to_str(c->signature_check.trust_level));
       			break;
       		default:
       			return 0;


 gpg-interface.c | 28 ++++++++++++++++++++++------
 gpg-interface.h |  8 ++++++++
 pretty.c        | 18 +-----------------
 3 files changed, 31 insertions(+), 23 deletions(-)

diff --git a/gpg-interface.c b/gpg-interface.c
index 947b58ad4da..6dff2414603 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -165,15 +165,17 @@ static struct {
 	{ 0, "TRUST_", GPG_STATUS_TRUST_LEVEL },
 };
 
-static struct {
+/* Keep the order same as enum signature_trust_level */
+static struct sigcheck_gpg_trust_level {
 	const char *key;
+	const char *display_key;
 	enum signature_trust_level value;
 } sigcheck_gpg_trust_level[] = {
-	{ "UNDEFINED", TRUST_UNDEFINED },
-	{ "NEVER", TRUST_NEVER },
-	{ "MARGINAL", TRUST_MARGINAL },
-	{ "FULLY", TRUST_FULLY },
-	{ "ULTIMATE", TRUST_ULTIMATE },
+	{ "UNDEFINED", "undefined", TRUST_UNDEFINED },
+	{ "NEVER", "never", TRUST_NEVER },
+	{ "MARGINAL", "marginal", TRUST_MARGINAL },
+	{ "FULLY", "fully", TRUST_FULLY },
+	{ "ULTIMATE", "ultimate", TRUST_ULTIMATE },
 };
 
 static void replace_cstring(char **field, const char *line, const char *next)
@@ -905,6 +907,20 @@ const char *get_signing_key(void)
 	return git_committer_info(IDENT_STRICT | IDENT_NO_DATE);
 }
 
+const char *gpg_trust_level_to_str(enum signature_trust_level level)
+{
+	struct sigcheck_gpg_trust_level *trust;
+
+	if (level < 0 || level >= ARRAY_SIZE(sigcheck_gpg_trust_level))
+		BUG("invalid trust level requested %d", level);
+
+	trust = &sigcheck_gpg_trust_level[level];
+	if (trust->value != level)
+		BUG("sigcheck_gpg_trust_level[] unsorted");
+
+	return sigcheck_gpg_trust_level[level].display_key;
+}
+
 int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key)
 {
 	return use_format->sign_buffer(buffer, signature, signing_key);
diff --git a/gpg-interface.h b/gpg-interface.h
index b30cbdcd3da..8a9ef41779e 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -71,6 +71,14 @@ size_t parse_signed_buffer(const char *buf, size_t size);
 int sign_buffer(struct strbuf *buffer, struct strbuf *signature,
 		const char *signing_key);
 
+
+/*
+ * Returns corresponding string in lowercase for a given member of
+ * enum signature_trust_level. For example, `TRUST_ULTIMATE` will
+ * return "ultimate".
+ */
+const char *gpg_trust_level_to_str(enum signature_trust_level level);
+
 int git_gpg_config(const char *, const char *, void *);
 void set_signing_key(const char *);
 const char *get_signing_key(void);
diff --git a/pretty.c b/pretty.c
index ee6114e3f0a..6d819103fbf 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1575,23 +1575,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 				strbuf_addstr(sb, c->signature_check.primary_key_fingerprint);
 			break;
 		case 'T':
-			switch (c->signature_check.trust_level) {
-			case TRUST_UNDEFINED:
-				strbuf_addstr(sb, "undefined");
-				break;
-			case TRUST_NEVER:
-				strbuf_addstr(sb, "never");
-				break;
-			case TRUST_MARGINAL:
-				strbuf_addstr(sb, "marginal");
-				break;
-			case TRUST_FULLY:
-				strbuf_addstr(sb, "fully");
-				break;
-			case TRUST_ULTIMATE:
-				strbuf_addstr(sb, "ultimate");
-				break;
-			}
+			strbuf_addstr(sb, gpg_trust_level_to_str(c->signature_check.trust_level));
 			break;
 		default:
 			return 0;

base-commit: 30cc8d0f147546d4dd77bf497f4dec51e7265bd8
-- 
gitgitgadget

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

[Junio C Hamano]

"Jaydeep Das via GitGitGadget" <gitgitgadget@gmail.com> writes:

>      @@ pretty.c: static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
>       -				strbuf_addstr(sb, "ultimate");
>       -				break;
>       -			}
>      -+			sig_str = gpg_trust_level_to_str(c->signature_check.trust_level);
>      -+			if (sig_str)
>      -+				strbuf_addstr(sb, sig_str);
>      -+			free(sig_str);
>      ++			strbuf_addstr(sb, gpg_trust_level_to_str(c->signature_check.trust_level));

This part of the range-diff is the most pleasant to see.

Will queue.  Thanks.