On Tue, 5 Apr 2011 12:20:52 -0500 Steve French <smfrench@xxxxxxxxx> wrote: > On Tue, Apr 5, 2011 at 12:17 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote: > > On Tue, 5 Apr 2011 12:06:59 -0500 > > Steve French <smfrench@xxxxxxxxx> wrote: > > > >> This is fine, but what about this alternative (initializing variables > >> can make code easier to read, and given dumb compiler warnings > >> sometimes better to leave in variable initialization ...): > >> > >> stevef@stevef-laptop:~/cifs-2.6$ git diff -a > >> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c > >> index 92e33d4..e4993a2 100644 > >> --- a/fs/cifs/cifssmb.c > >> +++ b/fs/cifs/cifssmb.c > >> @@ -117,7 +117,7 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command) > >> Â Â Â Â Â* calling routine > >> Â Â Â Â Â*/ > >> Â Â Â Â if (!tcon) > >> - Â Â Â Â Â Â Â return 0; > >> + Â Â Â Â Â Â Â return rc; > >> > >> Â Â Â Â ses = tcon->ses; > >> Â Â Â Â server = ses->server; > >> > >> > > > > So instead of eliminating a useless variable initialization, we instead > > manufacture a use for it? That seems sort of counter-productive, no? > > since we do "return rc" later in the function, why wouldn't we do the > same earlier > in the function for consistency? With rc initialized it is a bit easier to read. > In those places, we're returning rc because we called a function and stored the return code from it in rc. You certainly could "return rc" solely (and possibly use a "goto out" and have a single exit point for the function). The above patch doesn't do that though, it only changes the first spot where it does a "return 0". If you want to make that change then you probably ought to change the other places in the function for consistency. Again though, it seems pointless to me. Leaving it unintialized may help the compiler catch places where we "return rc" without setting it intentionally. -- Jeff Layton <jlayton@xxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
