I'm looking at a 2008 nfs-utils clean up done in commit 7a817c45. It
has this hunk in it:
@@ -111,7 +111,7 @@ void auth_unix_ip(FILE *f)
else if (client)
qword_print(f, *client?client:"DEFAULT");
qword_eol(f);
- xlog(D_CALL, "auth_unix_ip: client %p '%s'", client, *client?
client: "DEFAULT");
+ xlog(D_CALL, "auth_unix_ip: client %p '%s'", client, client?
client: "DEFAULT");
if (client) free(client);
free(he);
You changed the '*client ? client : "DEFAULT"' expression in the
xlog() call, but not in the qword_print() call right above it. Which
of these is correct, and why do they need to be different from each
other?
Seems to me _both_ should be 'client ? client : "DEFAULT"' (ie.
without the dereference).
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
On Thu, 2009-08-27 at 15:34 -0400, Chuck Lever wrote:
> I'm looking at a 2008 nfs-utils clean up done in commit 7a817c45. It
> has this hunk in it:
>
> @@ -111,7 +111,7 @@ void auth_unix_ip(FILE *f)
> else if (client)
> qword_print(f, *client?client:"DEFAULT");
> qword_eol(f);
> - xlog(D_CALL, "auth_unix_ip: client %p '%s'", client, *client?
> client: "DEFAULT");
> + xlog(D_CALL, "auth_unix_ip: client %p '%s'", client, client?
> client: "DEFAULT");
>
> if (client) free(client);
> free(he);
>
> You changed the '*client ? client : "DEFAULT"' expression in the
> xlog() call, but not in the qword_print() call right above it. Which
> of these is correct, and why do they need to be different from each
> other?
>
> Seems to me _both_ should be 'client ? client : "DEFAULT"' (ie.
> without the dereference).
There would be no point to not dereferencing client, it has already been
checked for NULL by "else if (client)". What it is doing is if the
client string is empty, it is printing "DEFAULT", and if the client
string is NULL, then it prints nothing.
Perhaps the xlog should be:
if (client)
xlog(D_CALL, "auth_unix_ip: client %p '%s', client, *client?client: "DEFAULT");
else
xlog(D_CALL, "auth_unix_ip: client NULL");
Frank
On Aug 27, 2009, at 6:01 PM, Frank Filz wrote:
> On Thu, 2009-08-27 at 15:34 -0400, Chuck Lever wrote:
>> I'm looking at a 2008 nfs-utils clean up done in commit 7a817c45. It
>> has this hunk in it:
>>
>> @@ -111,7 +111,7 @@ void auth_unix_ip(FILE *f)
>> else if (client)
>> qword_print(f, *client?client:"DEFAULT");
>> qword_eol(f);
>> - xlog(D_CALL, "auth_unix_ip: client %p '%s'", client, *client?
>> client: "DEFAULT");
>> + xlog(D_CALL, "auth_unix_ip: client %p '%s'", client, client?
>> client: "DEFAULT");
>>
>> if (client) free(client);
>> free(he);
>>
>> You changed the '*client ? client : "DEFAULT"' expression in the
>> xlog() call, but not in the qword_print() call right above it. Which
>> of these is correct, and why do they need to be different from each
>> other?
>>
>> Seems to me _both_ should be 'client ? client : "DEFAULT"' (ie.
>> without the dereference).
>
> There would be no point to not dereferencing client, it has already
> been
> checked for NULL by "else if (client)". What it is doing is if the
> client string is empty, it is printing "DEFAULT", and if the client
> string is NULL, then it prints nothing.
>
> Perhaps the xlog should be:
>
>
> if (client)
> xlog(D_CALL, "auth_unix_ip: client %p '%s', client, *client?
> client: "DEFAULT");
> else
> xlog(D_CALL, "auth_unix_ip: client NULL");
To confirm, then, the fix in 7a817c45 is bogus?
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
On Fri, 2009-08-28 at 09:43 -0400, Chuck Lever wrote:
> On Aug 27, 2009, at 6:01 PM, Frank Filz wrote:
> > On Thu, 2009-08-27 at 15:34 -0400, Chuck Lever wrote:
> >> I'm looking at a 2008 nfs-utils clean up done in commit 7a817c45. It
> >> has this hunk in it:
> >>
> >> @@ -111,7 +111,7 @@ void auth_unix_ip(FILE *f)
> >> else if (client)
> >> qword_print(f, *client?client:"DEFAULT");
> >> qword_eol(f);
> >> - xlog(D_CALL, "auth_unix_ip: client %p '%s'", client, *client?
> >> client: "DEFAULT");
> >> + xlog(D_CALL, "auth_unix_ip: client %p '%s'", client, client?
> >> client: "DEFAULT");
> >>
> >> if (client) free(client);
> >> free(he);
> >>
> >> You changed the '*client ? client : "DEFAULT"' expression in the
> >> xlog() call, but not in the qword_print() call right above it. Which
> >> of these is correct, and why do they need to be different from each
> >> other?
> >>
> >> Seems to me _both_ should be 'client ? client : "DEFAULT"' (ie.
> >> without the dereference).
> >
> > There would be no point to not dereferencing client, it has already
> > been
> > checked for NULL by "else if (client)". What it is doing is if the
> > client string is empty, it is printing "DEFAULT", and if the client
> > string is NULL, then it prints nothing.
> >
> > Perhaps the xlog should be:
> >
> >
> > if (client)
> > xlog(D_CALL, "auth_unix_ip: client %p '%s', client, *client?
> > client: "DEFAULT");
> > else
> > xlog(D_CALL, "auth_unix_ip: client NULL");
>
> To confirm, then, the fix in 7a817c45 is bogus?
>From what I can see, yes.
Frank