Re: [PATCH 1/5] gpg-interface: conditionally show the result in print_signature_buffer()

[Junio C Hamano <gitster@pobox.com>]

Hans Jerry Illikainen <hji@dyntopia.com> writes:

> The print_signature_buffer() function in gpg-interface.c is used to
> print the result of a GPG verified payload.  It takes a 'flags'
> parameter that determines what to print.
>
> Previously, the 'flags' parameter processed 2 flags:
>
> - GPG_VERIFY_RAW: to print the raw output from GPG instead of the
>   human(ish)-readable output.  One of these outputs were always
>   shown, irregardless of any other flags.
> - GPG_VERIFY_VERBOSE: to print the payload that was verified
>
> Interestingly, there was also a third flag defined in gpg-interface.h;
> GPG_VERIFY_OMIT_STATUS.  That flag wasn't used by the print function
> itself -- instead, callers would check for the presence of
> GPG_VERIFY_OMIT_STATUS before invoking print_signature_buffer().
>
> It seems reasonable that the GPG interface should handle all flags
> related to how the result should be (or shouldn't be) shown.  This patch
> implements that behavior by removing GPG_VERIFY_OMIT_STATUS and adding
> GPG_VERIFY_FULL.  If neither GPG_VERIFY_FULL nor GPG_VERIFY_VERBOSE is
> present, then nothing is printed.  This allows callers to invoke
> print_signature_buffer() unconditionally.

So in short, VERIFY_FULL is equivalent to !OMIT_STATUS?

As the direct callers of "print" are not the ones that set up bits
in flags, I think the proposed change makes the API easier to use.

Will queue.  Thanks.

> Signed-off-by: Hans Jerry Illikainen <hji@dyntopia.com>
> ---
>  builtin/tag.c           | 4 ++--
>  builtin/verify-commit.c | 2 +-
>  builtin/verify-tag.c    | 4 ++--
>  gpg-interface.c         | 2 +-
>  gpg-interface.h         | 6 +++---
>  tag.c                   | 4 +---
>  6 files changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/builtin/tag.c b/builtin/tag.c
> index e0a4c25382..8489e220e8 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -112,10 +112,10 @@ static int verify_tag(const char *name, const char *ref,
>  {
>  	int flags;
>  	const struct ref_format *format = cb_data;
> -	flags = GPG_VERIFY_VERBOSE;
> +	flags = GPG_VERIFY_FULL | GPG_VERIFY_VERBOSE;
>  
>  	if (format->format)
> -		flags = GPG_VERIFY_OMIT_STATUS;
> +		flags = 0;
>  
>  	if (gpg_verify_tag(oid, name, flags))
>  		return -1;
> diff --git a/builtin/verify-commit.c b/builtin/verify-commit.c
> index 40c69a0bed..2a099ec6ba 100644
> --- a/builtin/verify-commit.c
> +++ b/builtin/verify-commit.c
> @@ -63,7 +63,7 @@ static int git_verify_commit_config(const char *var, const char *value, void *cb
>  int cmd_verify_commit(int argc, const char **argv, const char *prefix)
>  {
>  	int i = 1, verbose = 0, had_error = 0;
> -	unsigned flags = 0;
> +	unsigned flags = GPG_VERIFY_FULL;
>  	const struct option verify_commit_options[] = {
>  		OPT__VERBOSE(&verbose, N_("print commit contents")),
>  		OPT_BIT(0, "raw", &flags, N_("print raw gpg status output"), GPG_VERIFY_RAW),
> diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
> index f45136a06b..bd5e99925b 100644
> --- a/builtin/verify-tag.c
> +++ b/builtin/verify-tag.c
> @@ -30,7 +30,7 @@ static int git_verify_tag_config(const char *var, const char *value, void *cb)
>  int cmd_verify_tag(int argc, const char **argv, const char *prefix)
>  {
>  	int i = 1, verbose = 0, had_error = 0;
> -	unsigned flags = 0;
> +	unsigned flags = GPG_VERIFY_FULL;
>  	struct ref_format format = REF_FORMAT_INIT;
>  	const struct option verify_tag_options[] = {
>  		OPT__VERBOSE(&verbose, N_("print tag contents")),
> @@ -53,7 +53,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)
>  		if (verify_ref_format(&format))
>  			usage_with_options(verify_tag_usage,
>  					   verify_tag_options);
> -		flags |= GPG_VERIFY_OMIT_STATUS;
> +		flags = 0;
>  	}
>  
>  	while (i < argc) {
> diff --git a/gpg-interface.c b/gpg-interface.c
> index 2d538bcd6e..fc182d39be 100644
> --- a/gpg-interface.c
> +++ b/gpg-interface.c
> @@ -341,7 +341,7 @@ void print_signature_buffer(const struct signature_check *sigc, unsigned flags)
>  	if (flags & GPG_VERIFY_VERBOSE && sigc->payload)
>  		fputs(sigc->payload, stdout);
>  
> -	if (output)
> +	if (flags & GPG_VERIFY_FULL && output)
>  		fputs(output, stderr);
>  }
>  
> diff --git a/gpg-interface.h b/gpg-interface.h
> index f4e9b4f371..4631a91330 100644
> --- a/gpg-interface.h
> +++ b/gpg-interface.h
> @@ -3,9 +3,9 @@
>  
>  struct strbuf;
>  
> -#define GPG_VERIFY_VERBOSE		1
> -#define GPG_VERIFY_RAW			2
> -#define GPG_VERIFY_OMIT_STATUS	4
> +#define GPG_VERIFY_VERBOSE (1 << 0)
> +#define GPG_VERIFY_RAW (1 << 1)
> +#define GPG_VERIFY_FULL (1 << 2)
>  
>  enum signature_trust_level {
>  	TRUST_UNDEFINED,
> diff --git a/tag.c b/tag.c
> index 71b544467e..b8d6da81eb 100644
> --- a/tag.c
> +++ b/tag.c
> @@ -28,9 +28,7 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)
>  
>  	ret = check_signature(buf, payload_size, buf + payload_size,
>  				size - payload_size, &sigc);
> -
> -	if (!(flags & GPG_VERIFY_OMIT_STATUS))
> -		print_signature_buffer(&sigc, flags);
> +	print_signature_buffer(&sigc, flags);
>  
>  	signature_check_clear(&sigc);
>  	return ret;