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

[Douglas Bagnall]

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') ||

which is less likely to confuse the future with something that really
ought to be simple.

Also it should be strtoul().

cheers,
Douglas