On Tue, 5 Apr 2011 12:20:52 -0500
Steve French <[email protected]> wrote:

> On Tue, Apr 5, 2011 at 12:17 PM, Jeff Layton <[email protected]> wrote:
> > On Tue, 5 Apr 2011 12:06:59 -0500
> > Steve French <[email protected]> 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 <[email protected]>
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to