Re: [PATCH] Pass "err" argument by address to "_nsError" function

[Jarkko Sakkinen <jarkko@kernel.org>]

On Mon, Feb 17, 2025 at 12:54:52AM +0000, Hazem Mohamed Abuelfotoh wrote:
> Commit 0d71523ab584 (“DNS: Support AFS SRV records and
> cell db config files”) has refactored the "nsError" function
> by moving some of error handling to "_nsError" function
> however we are passing the "err" argument to "_nsError"
> by value not by address which is wrong as that basically
> waste any processing we do in the "_nsError" function
> so correcting that by passing "err" by address.
> 
> Reported-by: Pratyush Yadav <ptyadav@amazon.com>
> Signed-off-by: Hazem Mohamed Abuelfotoh <abuehaze@amazon.com>
> ---
>  dns.afsdb.c        |  4 ++--
>  key.dns.h          |  2 +-
>  key.dns_resolver.c | 20 ++++++++++----------
>  3 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/dns.afsdb.c b/dns.afsdb.c
> index 986c0f3..7bffb60 100644
> --- a/dns.afsdb.c
> +++ b/dns.afsdb.c
> @@ -228,7 +228,7 @@ static int dns_query_AFSDB(const char *cell)
>  
>  	if (response_len < 0) {
>  		/* negative result */
> -		_nsError(h_errno, cell);
> +		_nsError(&h_errno, cell);
>  		return -1;
>  	}
>  
> @@ -267,7 +267,7 @@ static int dns_query_VL_SRV(const char *cell)
>  
>  	if (response_len < 0) {
>  		/* negative result */
> -		_nsError(h_errno, cell);
> +		_nsError(&h_errno, cell);
>  		return -1;
>  	}
>  
> diff --git a/key.dns.h b/key.dns.h
> index 33d0ab3..2fedbc3 100644
> --- a/key.dns.h
> +++ b/key.dns.h
> @@ -59,7 +59,7 @@ extern __attribute__((format(printf, 1, 2)))
>  void info(const char *fmt, ...);
>  extern __attribute__((noreturn))
>  void nsError(int err, const char *domain);
> -extern void _nsError(int err, const char *domain);
> +extern void _nsError(int *err, const char *domain);

Why a function declaration need extern anyway?

You could do w/o.

>  extern __attribute__((format(printf, 1, 2)))
>  void debug(const char *fmt, ...);
>  
> diff --git a/key.dns_resolver.c b/key.dns_resolver.c
> index 7a7ec42..6b16427 100644
> --- a/key.dns_resolver.c
> +++ b/key.dns_resolver.c
> @@ -157,19 +157,20 @@ static const int ns_errno_map[] = {
>  	[NO_DATA]		= ENODATA,
>  };
>  
> -void _nsError(int err, const char *domain)
> +void _nsError(int *err, const char *domain)
>  {
>  	if (isatty(2))
> -		fprintf(stderr, "NS:%s: %s.\n", domain, hstrerror(err));
> +		fprintf(stderr, "NS:%s: %s.\n", domain, hstrerror(*err));
>  	else
> -		syslog(LOG_INFO, "%s: %s", domain, hstrerror(err));
> +		syslog(LOG_INFO, "%s: %s", domain, hstrerror(*err));
>  
> -	if (err >= sizeof(ns_errno_map) / sizeof(ns_errno_map[0]))
> -		err = ECONNREFUSED;
> -	else
> -		err = ns_errno_map[err];
> +	if (*err >= sizeof(ns_errno_map) / sizeof(ns_errno_map[0]))
> +		*err = ECONNREFUSED;
> +	else{
> +		*err = ns_errno_map[*err];
> +	}
>  
> -	info("Reject the key with error %d", err);
> +	info("Reject the key with error %d", *err);
>  }
>  
>  void nsError(int err, const char *domain)
> @@ -177,8 +178,7 @@ void nsError(int err, const char *domain)
>  	unsigned timeout;
>  	int ret;
>  
> -	_nsError(err, domain);
> -
> +	_nsError(&err, domain);
>  	switch (err) {
>  	case TRY_AGAIN:
>  		timeout = 1;
> -- 
> 2.47.1
> 
> 

BR, Jarkko