2013/5/23 Jeff Layton <[email protected]>: > These look pretty cargo-culty to me, but let's be certain. Leave > them in place for now. Pop a WARN if it ever does happen. Also, > move to a more standard idiom for setting the "server" pointer. > > Signed-off-by: Jeff Layton <[email protected]> > --- > fs/cifs/cifssmb.c | 11 +++++------ > fs/cifs/sess.c | 4 +++- > fs/cifs/smb2pdu.c | 20 ++++++++------------ > 3 files changed, 16 insertions(+), 19 deletions(-) > > diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c > index a58dc77..c1c2006 100644 > --- a/fs/cifs/cifssmb.c > +++ b/fs/cifs/cifssmb.c > @@ -375,16 +375,15 @@ CIFSSMBNegotiate(const unsigned int xid, struct > cifs_ses *ses) > int rc = 0; > int bytes_returned; > int i; > - struct TCP_Server_Info *server; > + struct TCP_Server_Info *server = ses->server; > u16 count; > unsigned int secFlags; > > - if (ses->server) > - server = ses->server; > - else { > - rc = -EIO; > - return rc; > + if (!server) { > + WARN(1, "%s: server is NULL!\n", __func__); > + return -EIO; > } > + > rc = smb_init(SMB_COM_NEGOTIATE, 0, NULL /* no tcon yet */ , > (void **) &pSMB, (void **) &pSMBr); > if (rc) > diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c > index 838e224..e8c5dc9 100644 > --- a/fs/cifs/sess.c > +++ b/fs/cifs/sess.c > @@ -576,8 +576,10 @@ CIFS_SessSetup(const unsigned int xid, struct cifs_ses > *ses, > u16 blob_len; > char *ntlmsspblob = NULL; > > - if (ses == NULL) > + if (ses == NULL) { > + WARN(1, "%s: ses == NULL!", __func__); > return -EINVAL; > + } > > type = ses->server->secType; > cifs_dbg(FYI, "sess setup type %d\n", type); > diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c > index 2b95ce2..3af66aa 100644 > --- a/fs/cifs/smb2pdu.c > +++ b/fs/cifs/smb2pdu.c > @@ -328,7 +328,7 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses > *ses) > struct kvec iov[1]; > int rc = 0; > int resp_buftype; > - struct TCP_Server_Info *server; > + struct TCP_Server_Info *server = ses->server; > unsigned int sec_flags; > u16 temp = 0; > int blob_offset, blob_length; > @@ -337,11 +337,9 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses > *ses) > > cifs_dbg(FYI, "Negotiate protocol\n"); > > - if (ses->server) > - server = ses->server; > - else { > - rc = -EIO; > - return rc; > + if (!server) { > + WARN(1, "%s: server is NULL!\n", __func__); > + return -EIO; > } > > rc = small_smb2_init(SMB2_NEGOTIATE, NULL, (void **) &req); > @@ -480,7 +478,7 @@ SMB2_sess_setup(const unsigned int xid, struct cifs_ses > *ses, > int rc = 0; > int resp_buftype; > __le32 phase = NtLmNegotiate; /* NTLMSSP, if needed, is multistage */ > - struct TCP_Server_Info *server; > + struct TCP_Server_Info *server = ses->server; > unsigned int sec_flags; > u8 temp = 0; > u16 blob_length = 0; > @@ -490,11 +488,9 @@ SMB2_sess_setup(const unsigned int xid, struct cifs_ses > *ses, > > cifs_dbg(FYI, "Session Setup\n"); > > - if (ses->server) > - server = ses->server; > - else { > - rc = -EIO; > - return rc; > + if (!server) { > + WARN(1, "%s: server is NULL!\n", __func__); > + return -EIO; > } > > /* > -- > 1.8.1.4 > > -- > 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
Agreed. Also we have such checks several other places - may be it's good to make them raising a warning too? Reviewed-by: Pavel Shilovsky <[email protected]> -- Best regards, Pavel Shilovsky. -- 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
