[PATCH] nss_wrappe fix comparison between pointer and '\0'

[Douglas Bagnall]

hi Andreas,

On 16/08/17 21:35, Andreas Schneider wrote:
> On Wednesday, 16 August 2017 01:12:54 CEST Douglas Bagnall via samba-technical 
> wrote:
>> hi Gary,
>>
>> On 16/08/17 07:04, Gary Lockyer via samba-technical wrote:
>>> fixes GCC 7.1 warning, pointers being compared to '\0' without a
>>> dereference.
>>>
>>> Review appreciated.
>> I actually found this a while ago and Andreas reminded me to patch
>> against the upstream nss_wrapper repository:
>>
>> https://lists.samba.org/archive/samba-technical/2017-July/121750.html
>>
>> I gave that task a non-urgent priority. It turns out I was waiting for
>> you to prompt me. My attached version is slightly different,
>> explicitly ignoring an empty string.
> Hi Gary and Douglas,
> 
> I've fixed that last week, but do not have a RB+ for the patch yet. I think 
> the if-check for strtol should be as in the attached patch.
> 
> 
> Do you agree?
> 

Not quite.

> 	Andreas
> 
> -- Andreas Schneider GPG-ID: CC014E3D Samba Team asn at samba.org www.samba.org
> 
> 
> 0001-nwrap-Fix-check-after-strtol.patch
> 
> 
> From 908ef658674d912cc611978e97984b7278260c33 Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Wed, 2 Aug 2017 13:32:08 +0200
> Subject: [PATCH] nwrap: Fix check after strtol()
> 
> Signed-off-by: Andreas Schneider <asn at samba.org>
> ---
>  src/nss_wrapper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/nss_wrapper.c b/src/nss_wrapper.c
> index 70a5989..1d7e2d5 100644
> --- a/src/nss_wrapper.c
> +++ b/src/nss_wrapper.c
> @@ -1577,7 +1577,7 @@ static void nwrap_init(void)
>  	env = getenv("NSS_WRAPPER_MAX_HOSTENTS");
>  	if (env != NULL) {
>  		max_hostents_tmp = (size_t)strtol(env, &endptr, 10);
> -		if (((env != '\0') && (endptr == '\0')) ||
> +		if ((env == endptr) ||
>  		    (max_hostents_tmp == 0)) {
>  			NWRAP_LOG(NWRAP_LOG_DEBUG,
>  				  "Error parsing NSS_WRAPPER_MAX_HOSTENTS "

Isn't the (env == endptr) redundant? It matches an empty string, in
which case strtol will return zero which is caught by the
(max_hostents_tmp == 0) case.

It also doesn't catch a case like NSS_WRAPPER_MAX_HOSTENTS="32xyz",
where strtol will return 32 and the bad value is indicated by
*endpointer being 'x'.

It should actually be strtoul, unless NSS_WRAPPER_MAX_HOSTENTS=-1 is
meant to be shorthand for pseudo-infinite max_hostents.



Douglas