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 -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
