Re: [PATCH 3/8] treewide: Silence cert-err33-c clang-tidy warnings for fprintf()

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 7409 bytes --]

On Fri, Oct 25, 2024 at 01:04:33AM +0200, Stefano Brivio wrote:
> We use fprintf() to print to standard output or standard error
> streams. If something gets truncated or there's an output error, we
> don't really want to try and report that, and at the same time it's
> not abnormal behaviour upon which we should terminate, either.
> 
> Just silence the warning with an ugly FPRINTF() variadic macro casting
> the fprintf() expressions to void.

Again, not sure why I'm not seeing this with my clang version, but regardless:

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> 
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> ---
>  conf.c | 46 +++++++++++++++++++++++-----------------------
>  log.c  |  6 +++---
>  util.h |  3 +++
>  3 files changed, 29 insertions(+), 26 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index 2122600..9af15fe 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -731,19 +731,19 @@ static unsigned int conf_ip6(unsigned int ifi, struct ip6_ctx *ip6)
>  static void usage(const char *name, FILE *f, int status)
>  {
>  	if (strstr(name, "pasta")) {
> -		fprintf(f, "Usage: %s [OPTION]... [COMMAND] [ARGS]...\n", name);
> -		fprintf(f, "       %s [OPTION]... PID\n", name);
> -		fprintf(f, "       %s [OPTION]... --netns [PATH|NAME]\n", name);
> -		fprintf(f,
> +		FPRINTF(f, "Usage: %s [OPTION]... [COMMAND] [ARGS]...\n", name);
> +		FPRINTF(f, "       %s [OPTION]... PID\n", name);
> +		FPRINTF(f, "       %s [OPTION]... --netns [PATH|NAME]\n", name);
> +		FPRINTF(f,
>  			"\n"
>  			"Without PID or --netns, run the given command or a\n"
>  			"default shell in a new network and user namespace, and\n"
>  			"connect it via pasta.\n");
>  	} else {
> -		fprintf(f, "Usage: %s [OPTION]...\n", name);
> +		FPRINTF(f, "Usage: %s [OPTION]...\n", name);
>  	}
>  
> -	fprintf(f,
> +	FPRINTF(f,
>  		"\n"
>  		"  -d, --debug		Be verbose\n"
>  		"      --trace		Be extra verbose, implies --debug\n"
> @@ -760,17 +760,17 @@ static void usage(const char *name, FILE *f, int status)
>  		"  --version		Show version and exit\n");
>  
>  	if (strstr(name, "pasta")) {
> -		fprintf(f,
> +		FPRINTF(f,
>  			"  -I, --ns-ifname NAME	namespace interface name\n"
>  			"    default: same interface name as external one\n");
>  	} else {
> -		fprintf(f,
> +		FPRINTF(f,
>  			"  -s, --socket PATH	UNIX domain socket path\n"
>  			"    default: probe free path starting from "
>  			UNIX_SOCK_PATH "\n", 1);
>  	}
>  
> -	fprintf(f,
> +	FPRINTF(f,
>  		"  -F, --fd FD		Use FD as pre-opened connected socket\n"
>  		"  -p, --pcap FILE	Log tap-facing traffic to pcap file\n"
>  		"  -P, --pid FILE	Write own PID to the given file\n"
> @@ -801,28 +801,28 @@ static void usage(const char *name, FILE *f, int status)
>  		"    can be specified multiple times\n"
>  		"    a single, empty option disables DNS information\n");
>  	if (strstr(name, "pasta"))
> -		fprintf(f, "    default: don't use any addresses\n");
> +		FPRINTF(f, "    default: don't use any addresses\n");
>  	else
> -		fprintf(f, "    default: use addresses from /etc/resolv.conf\n");
> -	fprintf(f,
> +		FPRINTF(f, "    default: use addresses from /etc/resolv.conf\n");
> +	FPRINTF(f,
>  		"  -S, --search LIST	Space-separated list, search domains\n"
>  		"    a single, empty option disables the DNS search list\n");
>  	if (strstr(name, "pasta"))
> -		fprintf(f, "    default: don't use any search list\n");
> +		FPRINTF(f, "    default: don't use any search list\n");
>  	else
> -		fprintf(f, "    default: use search list from /etc/resolv.conf\n");
> +		FPRINTF(f, "    default: use search list from /etc/resolv.conf\n");
>  
>  	if (strstr(name, "pasta"))
> -		fprintf(f, "  --dhcp-dns	\tPass DNS list via DHCP/DHCPv6/NDP\n");
> +		FPRINTF(f, "  --dhcp-dns	\tPass DNS list via DHCP/DHCPv6/NDP\n");
>  	else
> -		fprintf(f, "  --no-dhcp-dns	No DNS list in DHCP/DHCPv6/NDP\n");
> +		FPRINTF(f, "  --no-dhcp-dns	No DNS list in DHCP/DHCPv6/NDP\n");
>  
>  	if (strstr(name, "pasta"))
> -		fprintf(f, "  --dhcp-search	Pass list via DHCP/DHCPv6/NDP\n");
> +		FPRINTF(f, "  --dhcp-search	Pass list via DHCP/DHCPv6/NDP\n");
>  	else
> -		fprintf(f, "  --no-dhcp-search	No list in DHCP/DHCPv6/NDP\n");
> +		FPRINTF(f, "  --no-dhcp-search	No list in DHCP/DHCPv6/NDP\n");
>  
> -	fprintf(f,
> +	FPRINTF(f,
>  		"  --map-host-loopback ADDR	Translate ADDR to refer to host\n"
>  	        "    can be specified zero to two times (for IPv4 and IPv6)\n"
>  		"    default: gateway address\n"
> @@ -850,7 +850,7 @@ static void usage(const char *name, FILE *f, int status)
>  	if (strstr(name, "pasta"))
>  		goto pasta_opts;
>  
> -	fprintf(f,
> +	FPRINTF(f,
>  		"  -1, --one-off	Quit after handling one single client\n"
>  		"  -t, --tcp-ports SPEC	TCP port forwarding to guest\n"
>  		"    can be specified multiple times\n"
> @@ -881,7 +881,7 @@ static void usage(const char *name, FILE *f, int status)
>  
>  pasta_opts:
>  
> -	fprintf(f,
> +	FPRINTF(f,
>  		"  -t, --tcp-ports SPEC	TCP port forwarding to namespace\n"
>  		"    can be specified multiple times\n"
>  		"    SPEC can be:\n"
> @@ -1419,9 +1419,9 @@ void conf(struct ctx *c, int argc, char **argv)
>  
>  			break;
>  		case 14:
> -			fprintf(stdout,
> +			FPRINTF(stdout,
>  				c->mode == MODE_PASTA ? "pasta " : "passt ");
> -			fprintf(stdout, VERSION_BLOB);
> +			FPRINTF(stdout, VERSION_BLOB);
>  			exit(EXIT_SUCCESS);
>  		case 15:
>  			ret = snprintf(c->ip4.ifname_out,
> diff --git a/log.c b/log.c
> index a61468e..6932885 100644
> --- a/log.c
> +++ b/log.c
> @@ -274,7 +274,7 @@ void vlogmsg(bool newline, bool cont, int pri, const char *format, va_list ap)
>  		char timestr[LOGTIME_STRLEN];
>  
>  		logtime_fmt(timestr, sizeof(timestr), now);
> -		fprintf(stderr, "%s: ", timestr);
> +		FPRINTF(stderr, "%s: ", timestr);
>  	}
>  
>  	if ((log_mask & LOG_MASK(LOG_PRI(pri))) || !log_conf_parsed) {
> @@ -293,7 +293,7 @@ void vlogmsg(bool newline, bool cont, int pri, const char *format, va_list ap)
>  	    (log_stderr && (log_mask & LOG_MASK(LOG_PRI(pri))))) {
>  		(void)vfprintf(stderr, format, ap);
>  		if (newline && format[strlen(format)] != '\n')
> -			fprintf(stderr, "\n");
> +			FPRINTF(stderr, "\n");
>  	}
>  }
>  
> @@ -399,7 +399,7 @@ void passt_vsyslog(bool newline, int pri, const char *format, va_list ap)
>  		n += snprintf(buf + n, BUFSIZ - n, "\n");
>  
>  	if (log_sock >= 0 && send(log_sock, buf, n, 0) != n && log_stderr)
> -		fprintf(stderr, "Failed to send %i bytes to syslog\n", n);
> +		FPRINTF(stderr, "Failed to send %i bytes to syslog\n", n);
>  }
>  
>  /**
> diff --git a/util.h b/util.h
> index 8449d00..d614094 100644
> --- a/util.h
> +++ b/util.h
> @@ -270,6 +270,9 @@ static inline bool mod_between(unsigned x, unsigned i, unsigned j, unsigned m)
>  	return mod_sub(x, i, m) < mod_sub(j, i, m);
>  }
>  
> +/* FPRINTF() intentionally silences cert-err33-c clang-tidy warnings */
> +#define FPRINTF(f, ...)	(void)fprintf(f, __VA_ARGS__)
> +
>  /*
>   * Workarounds for https://github.com/llvm/llvm-project/issues/58992
>   *

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]