[PATCH] nwrap: fix comparison, spotted by -Werror=pointer-compare

[Andreas Schneider]

On Monday, 28 August 2017 03:43:13 CEST Douglas Bagnall wrote:
> On 28/08/17 10:48, Andrew Bartlett via samba-technical wrote:
> > On Sun, 2017-08-27 at 21:08 +0200, Ralph Böhme via samba-technical
> > 
> > wrote:
> >> Hi!
> >> 
> >> Attached is a simple fix for nsswrapper. Ran into this after
> >> upgrading to Fedora
> >> 26. Please review&push if ok.
> > 
> > Check with Douglas on this.  He thinks this is a bit more subtle.
> 
> Ha, I think we're up to FIVE independent patches.
> 
> This version is obviously what was originally meant,...
> 
> >  	if (env != NULL) {
> >  	
> >  		max_hostents_tmp = (size_t)strtol(env, &endptr, 10);
> > 
> > -		if (((env != '\0') && (endptr == '\0')) ||
> > +		if (((env[0] != '\0') && (endptr[0] == '\0')) ||
> > 
> >  		    (max_hostents_tmp == 0)) {
> 
> ...but (endptr[0] == '\0') means the entire string was fully parsed,
> so we shouldn't be going into error case. The correct test is
> therefore (endptr[0] != '\0'). This guard expression (env[0] != '\0'),
> is saying the string is non-empty, which is also backwards. Really you
> 
> want to negate whole thing like this:
> > +		if (!((env[0] != '\0') && (endptr[0] == '\0')) ||
> 
> ...but that is full of redundancy and nested negation. In the case
> that (env[0] == '\0'), max_hostents_tmp will be set to zero, which we
> check anyway. So we can do without this mess and use
> 
> > +		if ((*endptr != '\0') ||

I think we want:

if (env == endptr || endptr[0] != '\0')

or

if (env == endptr || endptr[0] != '\0' || errno == ERANGE)



	Andreas

-- 
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             asn at samba.org
www.samba.org