Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760691Ab3JPPgr (ORCPT ); Wed, 16 Oct 2013 11:36:47 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36385 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757668Ab3JPPgo (ORCPT ); Wed, 16 Oct 2013 11:36:44 -0400 Date: Wed, 16 Oct 2013 11:36:32 -0400 From: Jeff Layton To: Tim Gardner Cc: linux-cifs@vger.kernel.org, samba-technical@lists.samba.org, linux-kernel@vger.kernel.org, Steve French Subject: Re: [PATCH 1/2 linux-next] cifs: Remove redundant multiplex identifier check from check_smb_hdr() Message-ID: <20131016113632.6548d399@tlielax.poochiereds.net> In-Reply-To: <1381936190-67628-1-git-send-email-timg@tpi.com> References: <1381936190-67628-1-git-send-email-timg@tpi.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2116 Lines: 66 On Wed, 16 Oct 2013 09:09:49 -0600 Tim Gardner wrote: > The only call site for check_smb_header() assigns 'mid' from the SMB > packet, which is then checked again in check_smb_header(). This seems > like redundant redundancy. > > Cc: Steve French > Signed-off-by: Tim Gardner > --- > fs/cifs/misc.c | 12 ++---------- > 1 file changed, 2 insertions(+), 10 deletions(-) > > diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c > index 138a011..298e31e 100644 > --- a/fs/cifs/misc.c > +++ b/fs/cifs/misc.c > @@ -278,7 +278,7 @@ header_assemble(struct smb_hdr *buffer, char smb_command /* command */ , > } > > static int > -check_smb_hdr(struct smb_hdr *smb, __u16 mid) > +check_smb_hdr(struct smb_hdr *smb) > { > /* does it have the right SMB "signature" ? */ > if (*(__le32 *) smb->Protocol != cpu_to_le32(0x424d53ff)) { > @@ -287,13 +287,6 @@ check_smb_hdr(struct smb_hdr *smb, __u16 mid) > return 1; > } > > - /* Make sure that message ids match */ > - if (mid != smb->Mid) { > - cifs_dbg(VFS, "Mids do not match. received=%u expected=%u\n", > - smb->Mid, mid); > - return 1; > - } > - > /* if it's a response then accept */ > if (smb->Flags & SMBFLG_RESPONSE) > return 0; > @@ -310,7 +303,6 @@ int > checkSMB(char *buf, unsigned int total_read) > { > struct smb_hdr *smb = (struct smb_hdr *)buf; > - __u16 mid = smb->Mid; > __u32 rfclen = be32_to_cpu(smb->smb_buf_length); > __u32 clc_len; /* calculated length */ > cifs_dbg(FYI, "checkSMB Length: 0x%x, smb_buf_length: 0x%x\n", > @@ -348,7 +340,7 @@ checkSMB(char *buf, unsigned int total_read) > } > > /* otherwise, there is enough to get to the BCC */ > - if (check_smb_hdr(smb, mid)) > + if (check_smb_hdr(smb)) > return -EIO; > clc_len = smbCalcSize(smb); > Nice... Reviewed-by: Jeff Layton -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/