[PATCH] cifs: remove unnecessary locking around sequence_number

[PATCH] cifs: remove unnecessary locking around sequence_number

[Jeff Layton]

The server->sequence_number is already protected by the srv_mutex. The
GlobalMid_lock is unneeded here.

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifsencrypt.c |    6 ++----
 fs/cifs/cifsglob.h    |    2 +-
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
index f856732..66f3d50 100644
--- a/fs/cifs/cifsencrypt.c
+++ b/fs/cifs/cifsencrypt.c
@@ -72,6 +72,7 @@ static int cifs_calculate_signature(const struct smb_hdr *cifs_pdu,
 	return 0;
 }
 
+/* must be called with server->srv_mutex held */
 int cifs_sign_smb(struct smb_hdr *cifs_pdu, struct TCP_Server_Info *server,
 		  __u32 *pexpected_response_sequence_number)
 {
@@ -84,14 +85,12 @@ int cifs_sign_smb(struct smb_hdr *cifs_pdu, struct TCP_Server_Info *server,
 	if ((cifs_pdu->Flags2 & SMBFLG2_SECURITY_SIGNATURE) == 0)
 		return rc;
 
-	spin_lock(&GlobalMid_Lock);
 	cifs_pdu->Signature.Sequence.SequenceNumber =
 			cpu_to_le32(server->sequence_number);
 	cifs_pdu->Signature.Sequence.Reserved = 0;
 
 	*pexpected_response_sequence_number = server->sequence_number++;
 	server->sequence_number++;
-	spin_unlock(&GlobalMid_Lock);
 
 	rc = cifs_calculate_signature(cifs_pdu, server, smb_signature);
 	if (rc)
@@ -149,6 +148,7 @@ static int cifs_calc_signature2(const struct kvec *iov, int n_vec,
 	return rc;
 }
 
+/* must be called with server->srv_mutex held */
 int cifs_sign_smb2(struct kvec *iov, int n_vec, struct TCP_Server_Info *server,
 		   __u32 *pexpected_response_sequence_number)
 {
@@ -162,14 +162,12 @@ int cifs_sign_smb2(struct kvec *iov, int n_vec, struct TCP_Server_Info *server,
 	if ((cifs_pdu->Flags2 & SMBFLG2_SECURITY_SIGNATURE) == 0)
 		return rc;
 
-	spin_lock(&GlobalMid_Lock);
 	cifs_pdu->Signature.Sequence.SequenceNumber =
 				cpu_to_le32(server->sequence_number);
 	cifs_pdu->Signature.Sequence.Reserved = 0;
 
 	*pexpected_response_sequence_number = server->sequence_number++;
 	server->sequence_number++;
-	spin_unlock(&GlobalMid_Lock);
 
 	rc = cifs_calc_signature2(iov, n_vec, server, smb_signature);
 	if (rc)
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index a8d9f32..bba731c 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -200,7 +200,7 @@ struct TCP_Server_Info {
 	char cryptkey[CIFS_CRYPTO_KEY_SIZE]; /* used by ntlm, ntlmv2 etc */
 	/* 16th byte of RFC1001 workstation name is always null */
 	char workstation_RFC1001_name[RFC1001_NAME_LEN_WITH_NULL];
-	__u32 sequence_number; /* needed for CIFS PDU signature */
+	__u32 sequence_number; /* for signing, protected by srv_mutex */
 	struct session_key session_key;
 	unsigned long lstrp; /* when we got last response from this server */
 	u16 dialect; /* dialect index that server chose */
-- 
1.7.3.2

[PATCH] cifs: remove unused CIFSSMBNotify

[Jeff Layton]

...and the global variables that only it references.

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifsfs.c    |    4 ---
 fs/cifs/cifsglob.h  |    5 ---
 fs/cifs/cifsproto.h |    4 ---
 fs/cifs/cifssmb.c   |   73 ---------------------------------------------------
 4 files changed, 0 insertions(+), 86 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 0ab2e2f..041d7c3 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -911,10 +911,6 @@ init_cifs(void)
 	int rc = 0;
 	cifs_proc_init();
 	INIT_LIST_HEAD(&cifs_tcp_ses_list);
-#ifdef CONFIG_CIFS_EXPERIMENTAL
-	INIT_LIST_HEAD(&GlobalDnotifyReqList);
-	INIT_LIST_HEAD(&GlobalDnotifyRsp_Q);
-#endif
 /*
  *  Initialize Global counters
  */
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index bba731c..b4c2524 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -735,11 +735,6 @@ GLOBAL_EXTERN spinlock_t		cifs_tcp_ses_lock;
  */
 GLOBAL_EXTERN spinlock_t	cifs_file_list_lock;
 
-/* Outstanding dir notify requests */
-GLOBAL_EXTERN struct list_head GlobalDnotifyReqList;
-/* DirNotify response queue */
-GLOBAL_EXTERN struct list_head GlobalDnotifyRsp_Q;
-
 /*
  * Global transaction id (XID) information
  */
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 6ed59af..7b0ca3a 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -383,10 +383,6 @@ extern int CIFSSMBCopy(int xid,
 			const char *toName, const int flags,
 			const struct nls_table *nls_codepage,
 			int remap_special_chars);
-extern int CIFSSMBNotify(const int xid, struct cifsTconInfo *tcon,
-			const int notify_subdirs, const __u16 netfid,
-			__u32 filter, struct file *file, int multishot,
-			const struct nls_table *nls_codepage);
 extern ssize_t CIFSSMBQAllEAs(const int xid, struct cifsTconInfo *tcon,
 			const unsigned char *searchName,
 			const unsigned char *ea_name, char *EAData,
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 2f2632b..0ef7c3a 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -5381,79 +5381,6 @@ setPermsRetry:
 	return rc;
 }
 
-int CIFSSMBNotify(const int xid, struct cifsTconInfo *tcon,
-		  const int notify_subdirs, const __u16 netfid,
-		  __u32 filter, struct file *pfile, int multishot,
-		  const struct nls_table *nls_codepage)
-{
-	int rc = 0;
-	struct smb_com_transaction_change_notify_req *pSMB = NULL;
-	struct smb_com_ntransaction_change_notify_rsp *pSMBr = NULL;
-	struct dir_notify_req *dnotify_req;
-	int bytes_returned;
-
-	cFYI(1, "In CIFSSMBNotify for file handle %d", (int)netfid);
-	rc = smb_init(SMB_COM_NT_TRANSACT, 23, tcon, (void **) &pSMB,
-		      (void **) &pSMBr);
-	if (rc)
-		return rc;
-
-	pSMB->TotalParameterCount = 0 ;
-	pSMB->TotalDataCount = 0;
-	pSMB->MaxParameterCount = cpu_to_le32(2);
-	/* BB find exact data count max from sess structure BB */
-	pSMB->MaxDataCount = 0; /* same in little endian or be */
-/* BB VERIFY verify which is correct for above BB */
-	pSMB->MaxDataCount = cpu_to_le32((tcon->ses->server->maxBuf -
-					     MAX_CIFS_HDR_SIZE) & 0xFFFFFF00);
-
-	pSMB->MaxSetupCount = 4;
-	pSMB->Reserved = 0;
-	pSMB->ParameterOffset = 0;
-	pSMB->DataCount = 0;
-	pSMB->DataOffset = 0;
-	pSMB->SetupCount = 4; /* single byte does not need le conversion */
-	pSMB->SubCommand = cpu_to_le16(NT_TRANSACT_NOTIFY_CHANGE);
-	pSMB->ParameterCount = pSMB->TotalParameterCount;
-	if (notify_subdirs)
-		pSMB->WatchTree = 1; /* one byte - no le conversion needed */
-	pSMB->Reserved2 = 0;
-	pSMB->CompletionFilter = cpu_to_le32(filter);
-	pSMB->Fid = netfid; /* file handle always le */
-	pSMB->ByteCount = 0;
-
-	rc = SendReceive(xid, tcon->ses, (struct smb_hdr *) pSMB,
-			 (struct smb_hdr *)pSMBr, &bytes_returned,
-			 CIFS_ASYNC_OP);
-	if (rc) {
-		cFYI(1, "Error in Notify = %d", rc);
-	} else {
-		/* Add file to outstanding requests */
-		/* BB change to kmem cache alloc */
-		dnotify_req = kmalloc(
-						sizeof(struct dir_notify_req),
-						 GFP_KERNEL);
-		if (dnotify_req) {
-			dnotify_req->Pid = pSMB->hdr.Pid;
-			dnotify_req->PidHigh = pSMB->hdr.PidHigh;
-			dnotify_req->Mid = pSMB->hdr.Mid;
-			dnotify_req->Tid = pSMB->hdr.Tid;
-			dnotify_req->Uid = pSMB->hdr.Uid;
-			dnotify_req->netfid = netfid;
-			dnotify_req->pfile = pfile;
-			dnotify_req->filter = filter;
-			dnotify_req->multishot = multishot;
-			spin_lock(&GlobalMid_Lock);
-			list_add_tail(&dnotify_req->lhead,
-					&GlobalDnotifyReqList);
-			spin_unlock(&GlobalMid_Lock);
-		} else
-			rc = -ENOMEM;
-	}
-	cifs_buf_release(pSMB);
-	return rc;
-}
-
 #ifdef CONFIG_CIFS_XATTR
 /*
  * Do a path-based QUERY_ALL_EAS call and parse the result. This is a common
-- 
1.7.3.2

[PATCH] cifs: remove Local_System_Name

[Jeff Layton]

...this string is zeroed out and nothing ever changes it.

Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/cifsfs.c   |    1 -
 fs/cifs/cifsglob.h |    2 --
 fs/cifs/connect.c  |   23 ++++++++++-------------
 3 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 041d7c3..57a7386 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -931,7 +931,6 @@ init_cifs(void)
 	GlobalCurrentXid = 0;
 	GlobalTotalActiveXid = 0;
 	GlobalMaxActiveXid = 0;
-	memset(Local_System_Name, 0, 15);
 	spin_lock_init(&cifs_tcp_ses_lock);
 	spin_lock_init(&cifs_file_list_lock);
 	spin_lock_init(&GlobalMid_Lock);
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index b4c2524..2ffa029 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -743,8 +743,6 @@ GLOBAL_EXTERN unsigned int GlobalTotalActiveXid; /* prot by GlobalMid_Sem */
 GLOBAL_EXTERN unsigned int GlobalMaxActiveXid;	/* prot by GlobalMid_Sem */
 GLOBAL_EXTERN spinlock_t GlobalMid_Lock;  /* protects above & list operations */
 					  /* on midQ entries */
-GLOBAL_EXTERN char Local_System_Name[15];
-
 /*
  *  Global counters, updated atomically
  */
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 251a17c..7758120 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -806,23 +806,20 @@ cifs_parse_mount_options(char *options, const char *devname,
 	short int override_gid = -1;
 	bool uid_specified = false;
 	bool gid_specified = false;
+	char *nodename = utsname()->nodename;
 
 	separator[0] = ',';
 	separator[1] = 0;
 
-	if (Local_System_Name[0] != 0)
-		memcpy(vol->source_rfc1001_name, Local_System_Name, 15);
-	else {
-		char *nodename = utsname()->nodename;
-		int n = strnlen(nodename, 15);
-		memset(vol->source_rfc1001_name, 0x20, 15);
-		for (i = 0; i < n; i++) {
-			/* does not have to be perfect mapping since field is
-			informational, only used for servers that do not support
-			port 445 and it can be overridden at mount time */
-			vol->source_rfc1001_name[i] = toupper(nodename[i]);
-		}
-	}
+	/*
+	 * does not have to be perfect mapping since field is
+	 * informational, only used for servers that do not support
+	 * port 445 and it can be overridden at mount time
+	 */
+	memset(vol->source_rfc1001_name, 0x20, 15);
+	for (i = 0; i < strnlen(nodename, 15); i++)
+		vol->source_rfc1001_name[i] = toupper(nodename[i]);
+
 	vol->source_rfc1001_name[15] = 0;
 	/* null target name indicates to use *SMBSERVR default called name
 	   if we end up sending RFC1001 session initialize */
-- 
1.7.3.2

Re: [PATCH] cifs: remove unused CIFSSMBNotify

[Shirish Pargaonkar]

On Mon, Nov 22, 2010 at 2:31 PM, Jeff Layton <jlayton-vpEMnDpepFuMZCB2o+C8xQ@public.gmane.org> wrote:
> ...and the global variables that only it references.
>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifsfs.c    |    4 ---
>  fs/cifs/cifsglob.h  |    5 ---
>  fs/cifs/cifsproto.h |    4 ---
>  fs/cifs/cifssmb.c   |   73 ---------------------------------------------------
>  4 files changed, 0 insertions(+), 86 deletions(-)
>
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index 0ab2e2f..041d7c3 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -911,10 +911,6 @@ init_cifs(void)
>        int rc = 0;
>        cifs_proc_init();
>        INIT_LIST_HEAD(&cifs_tcp_ses_list);
> -#ifdef CONFIG_CIFS_EXPERIMENTAL
> -       INIT_LIST_HEAD(&GlobalDnotifyReqList);
> -       INIT_LIST_HEAD(&GlobalDnotifyRsp_Q);
> -#endif
>  /*
>  *  Initialize Global counters
>  */
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index bba731c..b4c2524 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -735,11 +735,6 @@ GLOBAL_EXTERN spinlock_t           cifs_tcp_ses_lock;
>  */
>  GLOBAL_EXTERN spinlock_t       cifs_file_list_lock;
>
> -/* Outstanding dir notify requests */
> -GLOBAL_EXTERN struct list_head GlobalDnotifyReqList;
> -/* DirNotify response queue */
> -GLOBAL_EXTERN struct list_head GlobalDnotifyRsp_Q;
> -
>  /*
>  * Global transaction id (XID) information
>  */
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index 6ed59af..7b0ca3a 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -383,10 +383,6 @@ extern int CIFSSMBCopy(int xid,
>                        const char *toName, const int flags,
>                        const struct nls_table *nls_codepage,
>                        int remap_special_chars);
> -extern int CIFSSMBNotify(const int xid, struct cifsTconInfo *tcon,
> -                       const int notify_subdirs, const __u16 netfid,
> -                       __u32 filter, struct file *file, int multishot,
> -                       const struct nls_table *nls_codepage);
>  extern ssize_t CIFSSMBQAllEAs(const int xid, struct cifsTconInfo *tcon,
>                        const unsigned char *searchName,
>                        const unsigned char *ea_name, char *EAData,
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index 2f2632b..0ef7c3a 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -5381,79 +5381,6 @@ setPermsRetry:
>        return rc;
>  }
>
> -int CIFSSMBNotify(const int xid, struct cifsTconInfo *tcon,
> -                 const int notify_subdirs, const __u16 netfid,
> -                 __u32 filter, struct file *pfile, int multishot,
> -                 const struct nls_table *nls_codepage)
> -{
> -       int rc = 0;
> -       struct smb_com_transaction_change_notify_req *pSMB = NULL;
> -       struct smb_com_ntransaction_change_notify_rsp *pSMBr = NULL;
> -       struct dir_notify_req *dnotify_req;
> -       int bytes_returned;
> -
> -       cFYI(1, "In CIFSSMBNotify for file handle %d", (int)netfid);
> -       rc = smb_init(SMB_COM_NT_TRANSACT, 23, tcon, (void **) &pSMB,
> -                     (void **) &pSMBr);
> -       if (rc)
> -               return rc;
> -
> -       pSMB->TotalParameterCount = 0 ;
> -       pSMB->TotalDataCount = 0;
> -       pSMB->MaxParameterCount = cpu_to_le32(2);
> -       /* BB find exact data count max from sess structure BB */
> -       pSMB->MaxDataCount = 0; /* same in little endian or be */
> -/* BB VERIFY verify which is correct for above BB */
> -       pSMB->MaxDataCount = cpu_to_le32((tcon->ses->server->maxBuf -
> -                                            MAX_CIFS_HDR_SIZE) & 0xFFFFFF00);
> -
> -       pSMB->MaxSetupCount = 4;
> -       pSMB->Reserved = 0;
> -       pSMB->ParameterOffset = 0;
> -       pSMB->DataCount = 0;
> -       pSMB->DataOffset = 0;
> -       pSMB->SetupCount = 4; /* single byte does not need le conversion */
> -       pSMB->SubCommand = cpu_to_le16(NT_TRANSACT_NOTIFY_CHANGE);
> -       pSMB->ParameterCount = pSMB->TotalParameterCount;
> -       if (notify_subdirs)
> -               pSMB->WatchTree = 1; /* one byte - no le conversion needed */
> -       pSMB->Reserved2 = 0;
> -       pSMB->CompletionFilter = cpu_to_le32(filter);
> -       pSMB->Fid = netfid; /* file handle always le */
> -       pSMB->ByteCount = 0;
> -
> -       rc = SendReceive(xid, tcon->ses, (struct smb_hdr *) pSMB,
> -                        (struct smb_hdr *)pSMBr, &bytes_returned,
> -                        CIFS_ASYNC_OP);
> -       if (rc) {
> -               cFYI(1, "Error in Notify = %d", rc);
> -       } else {
> -               /* Add file to outstanding requests */
> -               /* BB change to kmem cache alloc */
> -               dnotify_req = kmalloc(
> -                                               sizeof(struct dir_notify_req),
> -                                                GFP_KERNEL);
> -               if (dnotify_req) {
> -                       dnotify_req->Pid = pSMB->hdr.Pid;
> -                       dnotify_req->PidHigh = pSMB->hdr.PidHigh;
> -                       dnotify_req->Mid = pSMB->hdr.Mid;
> -                       dnotify_req->Tid = pSMB->hdr.Tid;
> -                       dnotify_req->Uid = pSMB->hdr.Uid;
> -                       dnotify_req->netfid = netfid;
> -                       dnotify_req->pfile = pfile;
> -                       dnotify_req->filter = filter;
> -                       dnotify_req->multishot = multishot;
> -                       spin_lock(&GlobalMid_Lock);
> -                       list_add_tail(&dnotify_req->lhead,
> -                                       &GlobalDnotifyReqList);
> -                       spin_unlock(&GlobalMid_Lock);
> -               } else
> -                       rc = -ENOMEM;
> -       }
> -       cifs_buf_release(pSMB);
> -       return rc;
> -}
> -
>  #ifdef CONFIG_CIFS_XATTR
>  /*
>  * Do a path-based QUERY_ALL_EAS call and parse the result. This is a common
> --
> 1.7.3.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Do not think this function has been used for a long time.

Reviewed-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Re: [PATCH] cifs: remove unused CIFSSMBNotify

[Jeff Layton]

On Wed, 1 Dec 2010 07:55:54 -0600
Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> On Mon, Nov 22, 2010 at 2:31 PM, Jeff Layton <jlayton-vpEMnDpepFuMZCB2o+C8xQ@public.gmane.org> wrote:
> > ...and the global variables that only it references.
> >
> > Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > ---
> >  fs/cifs/cifsfs.c    |    4 ---
> >  fs/cifs/cifsglob.h  |    5 ---
> >  fs/cifs/cifsproto.h |    4 ---
> >  fs/cifs/cifssmb.c   |   73 ---------------------------------------------------
> >  4 files changed, 0 insertions(+), 86 deletions(-)
> >
> > diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> > index 0ab2e2f..041d7c3 100644
> > --- a/fs/cifs/cifsfs.c
> > +++ b/fs/cifs/cifsfs.c
> > @@ -911,10 +911,6 @@ init_cifs(void)
> >        int rc = 0;
> >        cifs_proc_init();
> >        INIT_LIST_HEAD(&cifs_tcp_ses_list);
> > -#ifdef CONFIG_CIFS_EXPERIMENTAL
> > -       INIT_LIST_HEAD(&GlobalDnotifyReqList);
> > -       INIT_LIST_HEAD(&GlobalDnotifyRsp_Q);
> > -#endif
> >  /*
> >  *  Initialize Global counters
> >  */
> > diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> > index bba731c..b4c2524 100644
> > --- a/fs/cifs/cifsglob.h
> > +++ b/fs/cifs/cifsglob.h
> > @@ -735,11 +735,6 @@ GLOBAL_EXTERN spinlock_t           cifs_tcp_ses_lock;
> >  */
> >  GLOBAL_EXTERN spinlock_t       cifs_file_list_lock;
> >
> > -/* Outstanding dir notify requests */
> > -GLOBAL_EXTERN struct list_head GlobalDnotifyReqList;
> > -/* DirNotify response queue */
> > -GLOBAL_EXTERN struct list_head GlobalDnotifyRsp_Q;
> > -
> >  /*
> >  * Global transaction id (XID) information
> >  */
> > diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> > index 6ed59af..7b0ca3a 100644
> > --- a/fs/cifs/cifsproto.h
> > +++ b/fs/cifs/cifsproto.h
> > @@ -383,10 +383,6 @@ extern int CIFSSMBCopy(int xid,
> >                        const char *toName, const int flags,
> >                        const struct nls_table *nls_codepage,
> >                        int remap_special_chars);
> > -extern int CIFSSMBNotify(const int xid, struct cifsTconInfo *tcon,
> > -                       const int notify_subdirs, const __u16 netfid,
> > -                       __u32 filter, struct file *file, int multishot,
> > -                       const struct nls_table *nls_codepage);
> >  extern ssize_t CIFSSMBQAllEAs(const int xid, struct cifsTconInfo *tcon,
> >                        const unsigned char *searchName,
> >                        const unsigned char *ea_name, char *EAData,
> > diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> > index 2f2632b..0ef7c3a 100644
> > --- a/fs/cifs/cifssmb.c
> > +++ b/fs/cifs/cifssmb.c
> > @@ -5381,79 +5381,6 @@ setPermsRetry:
> >        return rc;
> >  }
> >
> > -int CIFSSMBNotify(const int xid, struct cifsTconInfo *tcon,
> > -                 const int notify_subdirs, const __u16 netfid,
> > -                 __u32 filter, struct file *pfile, int multishot,
> > -                 const struct nls_table *nls_codepage)
> > -{
> > -       int rc = 0;
> > -       struct smb_com_transaction_change_notify_req *pSMB = NULL;
> > -       struct smb_com_ntransaction_change_notify_rsp *pSMBr = NULL;
> > -       struct dir_notify_req *dnotify_req;
> > -       int bytes_returned;
> > -
> > -       cFYI(1, "In CIFSSMBNotify for file handle %d", (int)netfid);
> > -       rc = smb_init(SMB_COM_NT_TRANSACT, 23, tcon, (void **) &pSMB,
> > -                     (void **) &pSMBr);
> > -       if (rc)
> > -               return rc;
> > -
> > -       pSMB->TotalParameterCount = 0 ;
> > -       pSMB->TotalDataCount = 0;
> > -       pSMB->MaxParameterCount = cpu_to_le32(2);
> > -       /* BB find exact data count max from sess structure BB */
> > -       pSMB->MaxDataCount = 0; /* same in little endian or be */
> > -/* BB VERIFY verify which is correct for above BB */
> > -       pSMB->MaxDataCount = cpu_to_le32((tcon->ses->server->maxBuf -
> > -                                            MAX_CIFS_HDR_SIZE) & 0xFFFFFF00);
> > -
> > -       pSMB->MaxSetupCount = 4;
> > -       pSMB->Reserved = 0;
> > -       pSMB->ParameterOffset = 0;
> > -       pSMB->DataCount = 0;
> > -       pSMB->DataOffset = 0;
> > -       pSMB->SetupCount = 4; /* single byte does not need le conversion */
> > -       pSMB->SubCommand = cpu_to_le16(NT_TRANSACT_NOTIFY_CHANGE);
> > -       pSMB->ParameterCount = pSMB->TotalParameterCount;
> > -       if (notify_subdirs)
> > -               pSMB->WatchTree = 1; /* one byte - no le conversion needed */
> > -       pSMB->Reserved2 = 0;
> > -       pSMB->CompletionFilter = cpu_to_le32(filter);
> > -       pSMB->Fid = netfid; /* file handle always le */
> > -       pSMB->ByteCount = 0;
> > -
> > -       rc = SendReceive(xid, tcon->ses, (struct smb_hdr *) pSMB,
> > -                        (struct smb_hdr *)pSMBr, &bytes_returned,
> > -                        CIFS_ASYNC_OP);
> > -       if (rc) {
> > -               cFYI(1, "Error in Notify = %d", rc);
> > -       } else {
> > -               /* Add file to outstanding requests */
> > -               /* BB change to kmem cache alloc */
> > -               dnotify_req = kmalloc(
> > -                                               sizeof(struct dir_notify_req),
> > -                                                GFP_KERNEL);
> > -               if (dnotify_req) {
> > -                       dnotify_req->Pid = pSMB->hdr.Pid;
> > -                       dnotify_req->PidHigh = pSMB->hdr.PidHigh;
> > -                       dnotify_req->Mid = pSMB->hdr.Mid;
> > -                       dnotify_req->Tid = pSMB->hdr.Tid;
> > -                       dnotify_req->Uid = pSMB->hdr.Uid;
> > -                       dnotify_req->netfid = netfid;
> > -                       dnotify_req->pfile = pfile;
> > -                       dnotify_req->filter = filter;
> > -                       dnotify_req->multishot = multishot;
> > -                       spin_lock(&GlobalMid_Lock);
> > -                       list_add_tail(&dnotify_req->lhead,
> > -                                       &GlobalDnotifyReqList);
> > -                       spin_unlock(&GlobalMid_Lock);
> > -               } else
> > -                       rc = -ENOMEM;
> > -       }
> > -       cifs_buf_release(pSMB);
> > -       return rc;
> > -}
> > -
> >  #ifdef CONFIG_CIFS_XATTR
> >  /*
> >  * Do a path-based QUERY_ALL_EAS call and parse the result. This is a common
> > --
> > 1.7.3.2
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> 
> Do not think this function has been used for a long time.
> 
> Reviewed-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

...not since the dir_notify stuff was ripped out of the VFS a couple
of years ago.

-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH] cifs: remove Local_System_Name

[Shirish Pargaonkar]

On Mon, Nov 22, 2010 at 2:31 PM, Jeff Layton <jlayton-vpEMnDpepFuMZCB2o+C8xQ@public.gmane.org> wrote:
> ...this string is zeroed out and nothing ever changes it.
>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifsfs.c   |    1 -
>  fs/cifs/cifsglob.h |    2 --
>  fs/cifs/connect.c  |   23 ++++++++++-------------
>  3 files changed, 10 insertions(+), 16 deletions(-)
>
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index 041d7c3..57a7386 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -931,7 +931,6 @@ init_cifs(void)
>        GlobalCurrentXid = 0;
>        GlobalTotalActiveXid = 0;
>        GlobalMaxActiveXid = 0;
> -       memset(Local_System_Name, 0, 15);
>        spin_lock_init(&cifs_tcp_ses_lock);
>        spin_lock_init(&cifs_file_list_lock);
>        spin_lock_init(&GlobalMid_Lock);
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index b4c2524..2ffa029 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -743,8 +743,6 @@ GLOBAL_EXTERN unsigned int GlobalTotalActiveXid; /* prot by GlobalMid_Sem */
>  GLOBAL_EXTERN unsigned int GlobalMaxActiveXid; /* prot by GlobalMid_Sem */
>  GLOBAL_EXTERN spinlock_t GlobalMid_Lock;  /* protects above & list operations */
>                                          /* on midQ entries */
> -GLOBAL_EXTERN char Local_System_Name[15];
> -
>  /*
>  *  Global counters, updated atomically
>  */
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 251a17c..7758120 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -806,23 +806,20 @@ cifs_parse_mount_options(char *options, const char *devname,
>        short int override_gid = -1;
>        bool uid_specified = false;
>        bool gid_specified = false;
> +       char *nodename = utsname()->nodename;
>
>        separator[0] = ',';
>        separator[1] = 0;
>
> -       if (Local_System_Name[0] != 0)
> -               memcpy(vol->source_rfc1001_name, Local_System_Name, 15);
> -       else {
> -               char *nodename = utsname()->nodename;
> -               int n = strnlen(nodename, 15);
> -               memset(vol->source_rfc1001_name, 0x20, 15);
> -               for (i = 0; i < n; i++) {
> -                       /* does not have to be perfect mapping since field is
> -                       informational, only used for servers that do not support
> -                       port 445 and it can be overridden at mount time */
> -                       vol->source_rfc1001_name[i] = toupper(nodename[i]);
> -               }
> -       }
> +       /*
> +        * does not have to be perfect mapping since field is
> +        * informational, only used for servers that do not support
> +        * port 445 and it can be overridden at mount time
> +        */
> +       memset(vol->source_rfc1001_name, 0x20, 15);
> +       for (i = 0; i < strnlen(nodename, 15); i++)
> +               vol->source_rfc1001_name[i] = toupper(nodename[i]);
> +
>        vol->source_rfc1001_name[15] = 0;
>        /* null target name indicates to use *SMBSERVR default called name
>           if we end up sending RFC1001 session initialize */
> --
> 1.7.3.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

We probably should define 15 or add an explanation for 15.

Re: [PATCH] cifs: remove Local_System_Name

[Jeff Layton]

On Wed, 1 Dec 2010 08:10:03 -0600
Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> On Mon, Nov 22, 2010 at 2:31 PM, Jeff Layton <jlayton-vpEMnDpepFuMZCB2o+C8xQ@public.gmane.org> wrote:
> > ...this string is zeroed out and nothing ever changes it.
> >
> > Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > ---
> >  fs/cifs/cifsfs.c   |    1 -
> >  fs/cifs/cifsglob.h |    2 --
> >  fs/cifs/connect.c  |   23 ++++++++++-------------
> >  3 files changed, 10 insertions(+), 16 deletions(-)
> >
> > diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> > index 041d7c3..57a7386 100644
> > --- a/fs/cifs/cifsfs.c
> > +++ b/fs/cifs/cifsfs.c
> > @@ -931,7 +931,6 @@ init_cifs(void)
> >        GlobalCurrentXid = 0;
> >        GlobalTotalActiveXid = 0;
> >        GlobalMaxActiveXid = 0;
> > -       memset(Local_System_Name, 0, 15);
> >        spin_lock_init(&cifs_tcp_ses_lock);
> >        spin_lock_init(&cifs_file_list_lock);
> >        spin_lock_init(&GlobalMid_Lock);
> > diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> > index b4c2524..2ffa029 100644
> > --- a/fs/cifs/cifsglob.h
> > +++ b/fs/cifs/cifsglob.h
> > @@ -743,8 +743,6 @@ GLOBAL_EXTERN unsigned int GlobalTotalActiveXid; /* prot by GlobalMid_Sem */
> >  GLOBAL_EXTERN unsigned int GlobalMaxActiveXid; /* prot by GlobalMid_Sem */
> >  GLOBAL_EXTERN spinlock_t GlobalMid_Lock;  /* protects above & list operations */
> >                                          /* on midQ entries */
> > -GLOBAL_EXTERN char Local_System_Name[15];
> > -
> >  /*
> >  *  Global counters, updated atomically
> >  */
> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> > index 251a17c..7758120 100644
> > --- a/fs/cifs/connect.c
> > +++ b/fs/cifs/connect.c
> > @@ -806,23 +806,20 @@ cifs_parse_mount_options(char *options, const char *devname,
> >        short int override_gid = -1;
> >        bool uid_specified = false;
> >        bool gid_specified = false;
> > +       char *nodename = utsname()->nodename;
> >
> >        separator[0] = ',';
> >        separator[1] = 0;
> >
> > -       if (Local_System_Name[0] != 0)
> > -               memcpy(vol->source_rfc1001_name, Local_System_Name, 15);
> > -       else {
> > -               char *nodename = utsname()->nodename;
> > -               int n = strnlen(nodename, 15);
> > -               memset(vol->source_rfc1001_name, 0x20, 15);
> > -               for (i = 0; i < n; i++) {
> > -                       /* does not have to be perfect mapping since field is
> > -                       informational, only used for servers that do not support
> > -                       port 445 and it can be overridden at mount time */
> > -                       vol->source_rfc1001_name[i] = toupper(nodename[i]);
> > -               }
> > -       }
> > +       /*
> > +        * does not have to be perfect mapping since field is
> > +        * informational, only used for servers that do not support
> > +        * port 445 and it can be overridden at mount time
> > +        */
> > +       memset(vol->source_rfc1001_name, 0x20, 15);
> > +       for (i = 0; i < strnlen(nodename, 15); i++)
> > +               vol->source_rfc1001_name[i] = toupper(nodename[i]);
> > +
> >        vol->source_rfc1001_name[15] = 0;
> >        /* null target name indicates to use *SMBSERVR default called name
> >           if we end up sending RFC1001 session initialize */
> > --
> > 1.7.3.2
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> 
> We probably should define 15 or add an explanation for 15.

Good point. I guess that should probably be RFC1001_NAME_LEN ? There
are a number of other places that use hardcoded "15" or "16" values
around this as well. Any objection to me fixing that in a separate
cleanup patch?

-- 
Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>

Re: [PATCH] cifs: remove unnecessary locking around sequence_number

[Shirish Pargaonkar]

On Mon, Nov 22, 2010 at 2:31 PM, Jeff Layton <jlayton-vpEMnDpepFuMZCB2o+C8xQ@public.gmane.org> wrote:
> The server->sequence_number is already protected by the srv_mutex. The
> GlobalMid_lock is unneeded here.
>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/cifsencrypt.c |    6 ++----
>  fs/cifs/cifsglob.h    |    2 +-
>  2 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
> index f856732..66f3d50 100644
> --- a/fs/cifs/cifsencrypt.c
> +++ b/fs/cifs/cifsencrypt.c
> @@ -72,6 +72,7 @@ static int cifs_calculate_signature(const struct smb_hdr *cifs_pdu,
>        return 0;
>  }
>
> +/* must be called with server->srv_mutex held */
>  int cifs_sign_smb(struct smb_hdr *cifs_pdu, struct TCP_Server_Info *server,
>                  __u32 *pexpected_response_sequence_number)
>  {
> @@ -84,14 +85,12 @@ int cifs_sign_smb(struct smb_hdr *cifs_pdu, struct TCP_Server_Info *server,
>        if ((cifs_pdu->Flags2 & SMBFLG2_SECURITY_SIGNATURE) == 0)
>                return rc;
>
> -       spin_lock(&GlobalMid_Lock);
>        cifs_pdu->Signature.Sequence.SequenceNumber =
>                        cpu_to_le32(server->sequence_number);
>        cifs_pdu->Signature.Sequence.Reserved = 0;
>
>        *pexpected_response_sequence_number = server->sequence_number++;
>        server->sequence_number++;
> -       spin_unlock(&GlobalMid_Lock);
>
>        rc = cifs_calculate_signature(cifs_pdu, server, smb_signature);
>        if (rc)
> @@ -149,6 +148,7 @@ static int cifs_calc_signature2(const struct kvec *iov, int n_vec,
>        return rc;
>  }
>
> +/* must be called with server->srv_mutex held */
>  int cifs_sign_smb2(struct kvec *iov, int n_vec, struct TCP_Server_Info *server,
>                   __u32 *pexpected_response_sequence_number)
>  {
> @@ -162,14 +162,12 @@ int cifs_sign_smb2(struct kvec *iov, int n_vec, struct TCP_Server_Info *server,
>        if ((cifs_pdu->Flags2 & SMBFLG2_SECURITY_SIGNATURE) == 0)
>                return rc;
>
> -       spin_lock(&GlobalMid_Lock);
>        cifs_pdu->Signature.Sequence.SequenceNumber =
>                                cpu_to_le32(server->sequence_number);
>        cifs_pdu->Signature.Sequence.Reserved = 0;
>
>        *pexpected_response_sequence_number = server->sequence_number++;
>        server->sequence_number++;
> -       spin_unlock(&GlobalMid_Lock);
>
>        rc = cifs_calc_signature2(iov, n_vec, server, smb_signature);
>        if (rc)
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index a8d9f32..bba731c 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -200,7 +200,7 @@ struct TCP_Server_Info {
>        char cryptkey[CIFS_CRYPTO_KEY_SIZE]; /* used by ntlm, ntlmv2 etc */
>        /* 16th byte of RFC1001 workstation name is always null */
>        char workstation_RFC1001_name[RFC1001_NAME_LEN_WITH_NULL];
> -       __u32 sequence_number; /* needed for CIFS PDU signature */
> +       __u32 sequence_number; /* for signing, protected by srv_mutex */
>        struct session_key session_key;
>        unsigned long lstrp; /* when we got last response from this server */
>        u16 dialect; /* dialect index that server chose */
> --
> 1.7.3.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Looks correct. Two less contenders for global mid lock.

Reviewed-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Re: [PATCH] cifs: remove Local_System_Name

[Shirish Pargaonkar]

On Wed, Dec 1, 2010 at 8:27 AM, Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> On Wed, 1 Dec 2010 08:10:03 -0600
> Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> On Mon, Nov 22, 2010 at 2:31 PM, Jeff Layton <jlayton-vpEMnDpepFvY2uhgrK1nQg@public.gmane.orgt> wrote:
>> > ...this string is zeroed out and nothing ever changes it.
>> >
>> > Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> > ---
>> >  fs/cifs/cifsfs.c   |    1 -
>> >  fs/cifs/cifsglob.h |    2 --
>> >  fs/cifs/connect.c  |   23 ++++++++++-------------
>> >  3 files changed, 10 insertions(+), 16 deletions(-)
>> >
>> > diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
>> > index 041d7c3..57a7386 100644
>> > --- a/fs/cifs/cifsfs.c
>> > +++ b/fs/cifs/cifsfs.c
>> > @@ -931,7 +931,6 @@ init_cifs(void)
>> >        GlobalCurrentXid = 0;
>> >        GlobalTotalActiveXid = 0;
>> >        GlobalMaxActiveXid = 0;
>> > -       memset(Local_System_Name, 0, 15);
>> >        spin_lock_init(&cifs_tcp_ses_lock);
>> >        spin_lock_init(&cifs_file_list_lock);
>> >        spin_lock_init(&GlobalMid_Lock);
>> > diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>> > index b4c2524..2ffa029 100644
>> > --- a/fs/cifs/cifsglob.h
>> > +++ b/fs/cifs/cifsglob.h
>> > @@ -743,8 +743,6 @@ GLOBAL_EXTERN unsigned int GlobalTotalActiveXid; /* prot by GlobalMid_Sem */
>> >  GLOBAL_EXTERN unsigned int GlobalMaxActiveXid; /* prot by GlobalMid_Sem */
>> >  GLOBAL_EXTERN spinlock_t GlobalMid_Lock;  /* protects above & list operations */
>> >                                          /* on midQ entries */
>> > -GLOBAL_EXTERN char Local_System_Name[15];
>> > -
>> >  /*
>> >  *  Global counters, updated atomically
>> >  */
>> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> > index 251a17c..7758120 100644
>> > --- a/fs/cifs/connect.c
>> > +++ b/fs/cifs/connect.c
>> > @@ -806,23 +806,20 @@ cifs_parse_mount_options(char *options, const char *devname,
>> >        short int override_gid = -1;
>> >        bool uid_specified = false;
>> >        bool gid_specified = false;
>> > +       char *nodename = utsname()->nodename;
>> >
>> >        separator[0] = ',';
>> >        separator[1] = 0;
>> >
>> > -       if (Local_System_Name[0] != 0)
>> > -               memcpy(vol->source_rfc1001_name, Local_System_Name, 15);
>> > -       else {
>> > -               char *nodename = utsname()->nodename;
>> > -               int n = strnlen(nodename, 15);
>> > -               memset(vol->source_rfc1001_name, 0x20, 15);
>> > -               for (i = 0; i < n; i++) {
>> > -                       /* does not have to be perfect mapping since field is
>> > -                       informational, only used for servers that do not support
>> > -                       port 445 and it can be overridden at mount time */
>> > -                       vol->source_rfc1001_name[i] = toupper(nodename[i]);
>> > -               }
>> > -       }
>> > +       /*
>> > +        * does not have to be perfect mapping since field is
>> > +        * informational, only used for servers that do not support
>> > +        * port 445 and it can be overridden at mount time
>> > +        */
>> > +       memset(vol->source_rfc1001_name, 0x20, 15);
>> > +       for (i = 0; i < strnlen(nodename, 15); i++)
>> > +               vol->source_rfc1001_name[i] = toupper(nodename[i]);
>> > +
>> >        vol->source_rfc1001_name[15] = 0;
>> >        /* null target name indicates to use *SMBSERVR default called name
>> >           if we end up sending RFC1001 session initialize */
>> > --
>> > 1.7.3.2
>> >
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
>> > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> >
>>
>> We probably should define 15 or add an explanation for 15.
>
> Good point. I guess that should probably be RFC1001_NAME_LEN ? There
> are a number of other places that use hardcoded "15" or "16" values
> around this as well. Any objection to me fixing that in a separate
> cleanup patch?
>
> --
> Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
>

RFC1001_NAME_LEN  sounds apt.
No objection.

Reviewed-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

[PATCH] cifs: replace some hardcoded values with preprocessor constants

[Jeff Layton]

A number of places that deal with RFC1001/1002 negotiations have bare
"15" or "16" values. Replace them with RFC_1001_NAME_LEN and
RFC_1001_NAME_LEN_WITH_NULL.

The patch also cleans up some checkpatch warnings for code surrounding
the changes. This should apply cleanly on top of the patch to remove
Local_System_Name.

Reported-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/cifs/connect.c |   37 +++++++++++++++++++------------------
 1 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 5d48e09..a96324b 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -64,8 +64,8 @@ struct smb_vol {
 	char *UNC;
 	char *UNCip;
 	char *iocharset;  /* local code page for mapping to and from Unicode */
-	char source_rfc1001_name[16]; /* netbios name of client */
-	char target_rfc1001_name[16]; /* netbios name of server for Win9x/ME */
+	char source_rfc1001_name[RFC1001_NAME_LEN_WITH_NULL]; /* clnt nb name */
+	char target_rfc1001_name[RFC1001_NAME_LEN_WITH_NULL]; /* srvr nb name */
 	uid_t cred_uid;
 	uid_t linux_uid;
 	gid_t linux_gid;
@@ -816,11 +816,11 @@ cifs_parse_mount_options(char *options, const char *devname,
 	 * informational, only used for servers that do not support
 	 * port 445 and it can be overridden at mount time
 	 */
-	memset(vol->source_rfc1001_name, 0x20, 15);
-	for (i = 0; i < strnlen(nodename, 15); i++)
+	memset(vol->source_rfc1001_name, 0x20, RFC1001_NAME_LEN);
+	for (i = 0; i < strnlen(nodename, RFC1001_NAME_LEN); i++)
 		vol->source_rfc1001_name[i] = toupper(nodename[i]);
 
-	vol->source_rfc1001_name[15] = 0;
+	vol->source_rfc1001_name[RFC1001_NAME_LEN] = 0;
 	/* null target name indicates to use *SMBSERVR default called name
 	   if we end up sending RFC1001 session initialize */
 	vol->target_rfc1001_name[0] = 0;
@@ -1165,22 +1165,22 @@ cifs_parse_mount_options(char *options, const char *devname,
 			if (!value || !*value || (*value == ' ')) {
 				cFYI(1, "invalid (empty) netbiosname");
 			} else {
-				memset(vol->source_rfc1001_name, 0x20, 15);
-				for (i = 0; i < 15; i++) {
-				/* BB are there cases in which a comma can be
-				valid in this workstation netbios name (and need
-				special handling)? */
-
-				/* We do not uppercase netbiosname for user */
+				memset(vol->source_rfc1001_name, 0x20,
+					RFC1001_NAME_LEN);
+				/*
+				 * FIXME: are there cases in which a comma can
+				 * be valid in workstation netbios name (and
+				 * need special handling)?
+				 */
+				for (i = 0; i < RFC1001_NAME_LEN; i++) {
+					/* don't ucase netbiosname for user */
 					if (value[i] == 0)
 						break;
-					else
-						vol->source_rfc1001_name[i] =
-								value[i];
+					vol->source_rfc1001_name[i] = value[i];
 				}
 				/* The string has 16th byte zero still from
 				set at top of the function  */
-				if ((i == 15) && (value[i] != 0))
+				if (i == RFC1001_NAME_LEN && value[i] != 0)
 					printk(KERN_WARNING "CIFS: netbiosname"
 						" longer than 15 truncated.\n");
 			}
@@ -1190,7 +1190,8 @@ cifs_parse_mount_options(char *options, const char *devname,
 				cFYI(1, "empty server netbiosname specified");
 			} else {
 				/* last byte, type, is 0x20 for servr type */
-				memset(vol->target_rfc1001_name, 0x20, 16);
+				memset(vol->target_rfc1001_name, 0x20,
+					RFC1001_NAME_LEN_WITH_NULL);
 
 				for (i = 0; i < 15; i++) {
 				/* BB are there cases in which a comma can be
@@ -1207,7 +1208,7 @@ cifs_parse_mount_options(char *options, const char *devname,
 				}
 				/* The string has 16th byte zero still from
 				   set at top of the function  */
-				if ((i == 15) && (value[i] != 0))
+				if (i == RFC1001_NAME_LEN && value[i] != 0)
 					printk(KERN_WARNING "CIFS: server net"
 					"biosname longer than 15 truncated.\n");
 			}
-- 
1.7.3.2

Re: [PATCH] cifs: replace some hardcoded values with preprocessor constants

[Shirish Pargaonkar]

On Wed, Dec 1, 2010 at 8:49 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> A number of places that deal with RFC1001/1002 negotiations have bare
> "15" or "16" values. Replace them with RFC_1001_NAME_LEN and
> RFC_1001_NAME_LEN_WITH_NULL.
>
> The patch also cleans up some checkpatch warnings for code surrounding
> the changes. This should apply cleanly on top of the patch to remove
> Local_System_Name.
>
> Reported-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/connect.c |   37 +++++++++++++++++++------------------
>  1 files changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 5d48e09..a96324b 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -64,8 +64,8 @@ struct smb_vol {
>        char *UNC;
>        char *UNCip;
>        char *iocharset;  /* local code page for mapping to and from Unicode */
> -       char source_rfc1001_name[16]; /* netbios name of client */
> -       char target_rfc1001_name[16]; /* netbios name of server for Win9x/ME */
> +       char source_rfc1001_name[RFC1001_NAME_LEN_WITH_NULL]; /* clnt nb name */
> +       char target_rfc1001_name[RFC1001_NAME_LEN_WITH_NULL]; /* srvr nb name */
>        uid_t cred_uid;
>        uid_t linux_uid;
>        gid_t linux_gid;
> @@ -816,11 +816,11 @@ cifs_parse_mount_options(char *options, const char *devname,
>         * informational, only used for servers that do not support
>         * port 445 and it can be overridden at mount time
>         */
> -       memset(vol->source_rfc1001_name, 0x20, 15);
> -       for (i = 0; i < strnlen(nodename, 15); i++)
> +       memset(vol->source_rfc1001_name, 0x20, RFC1001_NAME_LEN);
> +       for (i = 0; i < strnlen(nodename, RFC1001_NAME_LEN); i++)
>                vol->source_rfc1001_name[i] = toupper(nodename[i]);
>
> -       vol->source_rfc1001_name[15] = 0;
> +       vol->source_rfc1001_name[RFC1001_NAME_LEN] = 0;
>        /* null target name indicates to use *SMBSERVR default called name
>           if we end up sending RFC1001 session initialize */
>        vol->target_rfc1001_name[0] = 0;
> @@ -1165,22 +1165,22 @@ cifs_parse_mount_options(char *options, const char *devname,
>                        if (!value || !*value || (*value == ' ')) {
>                                cFYI(1, "invalid (empty) netbiosname");
>                        } else {
> -                               memset(vol->source_rfc1001_name, 0x20, 15);
> -                               for (i = 0; i < 15; i++) {
> -                               /* BB are there cases in which a comma can be
> -                               valid in this workstation netbios name (and need
> -                               special handling)? */
> -
> -                               /* We do not uppercase netbiosname for user */
> +                               memset(vol->source_rfc1001_name, 0x20,
> +                                       RFC1001_NAME_LEN);
> +                               /*
> +                                * FIXME: are there cases in which a comma can
> +                                * be valid in workstation netbios name (and
> +                                * need special handling)?
> +                                */
> +                               for (i = 0; i < RFC1001_NAME_LEN; i++) {
> +                                       /* don't ucase netbiosname for user */
>                                        if (value[i] == 0)
>                                                break;
> -                                       else
> -                                               vol->source_rfc1001_name[i] =
> -                                                               value[i];
> +                                       vol->source_rfc1001_name[i] = value[i];
>                                }
>                                /* The string has 16th byte zero still from
>                                set at top of the function  */
> -                               if ((i == 15) && (value[i] != 0))
> +                               if (i == RFC1001_NAME_LEN && value[i] != 0)
>                                        printk(KERN_WARNING "CIFS: netbiosname"
>                                                " longer than 15 truncated.\n");
>                        }
> @@ -1190,7 +1190,8 @@ cifs_parse_mount_options(char *options, const char *devname,
>                                cFYI(1, "empty server netbiosname specified");
>                        } else {
>                                /* last byte, type, is 0x20 for servr type */
> -                               memset(vol->target_rfc1001_name, 0x20, 16);
> +                               memset(vol->target_rfc1001_name, 0x20,
> +                                       RFC1001_NAME_LEN_WITH_NULL);
>
>                                for (i = 0; i < 15; i++) {
>                                /* BB are there cases in which a comma can be
> @@ -1207,7 +1208,7 @@ cifs_parse_mount_options(char *options, const char *devname,
>                                }
>                                /* The string has 16th byte zero still from
>                                   set at top of the function  */
> -                               if ((i == 15) && (value[i] != 0))
> +                               if (i == RFC1001_NAME_LEN && value[i] != 0)
>                                        printk(KERN_WARNING "CIFS: server net"
>                                        "biosname longer than 15 truncated.\n");
>                        }
> --
> 1.7.3.2
>
>

Jeff, I do not see where RFC1001_NAME_LEN_WITH_NULL and RFC1000_NAME_LEN
are defined.

Re: [PATCH] cifs: replace some hardcoded values with preprocessor constants

[Jeff Layton]

On Wed, 1 Dec 2010 09:03:32 -0600
Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> On Wed, Dec 1, 2010 at 8:49 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > A number of places that deal with RFC1001/1002 negotiations have bare
> > "15" or "16" values. Replace them with RFC_1001_NAME_LEN and
> > RFC_1001_NAME_LEN_WITH_NULL.
> >
> > The patch also cleans up some checkpatch warnings for code surrounding
> > the changes. This should apply cleanly on top of the patch to remove
> > Local_System_Name.
> >
> > Reported-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > ---
> >  fs/cifs/connect.c |   37 +++++++++++++++++++------------------
> >  1 files changed, 19 insertions(+), 18 deletions(-)
> >
> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> > index 5d48e09..a96324b 100644
> > --- a/fs/cifs/connect.c
> > +++ b/fs/cifs/connect.c
> > @@ -64,8 +64,8 @@ struct smb_vol {
> >        char *UNC;
> >        char *UNCip;
> >        char *iocharset;  /* local code page for mapping to and from Unicode */
> > -       char source_rfc1001_name[16]; /* netbios name of client */
> > -       char target_rfc1001_name[16]; /* netbios name of server for Win9x/ME */
> > +       char source_rfc1001_name[RFC1001_NAME_LEN_WITH_NULL]; /* clnt nb name */
> > +       char target_rfc1001_name[RFC1001_NAME_LEN_WITH_NULL]; /* srvr nb name */
> >        uid_t cred_uid;
> >        uid_t linux_uid;
> >        gid_t linux_gid;
> > @@ -816,11 +816,11 @@ cifs_parse_mount_options(char *options, const char *devname,
> >         * informational, only used for servers that do not support
> >         * port 445 and it can be overridden at mount time
> >         */
> > -       memset(vol->source_rfc1001_name, 0x20, 15);
> > -       for (i = 0; i < strnlen(nodename, 15); i++)
> > +       memset(vol->source_rfc1001_name, 0x20, RFC1001_NAME_LEN);
> > +       for (i = 0; i < strnlen(nodename, RFC1001_NAME_LEN); i++)
> >                vol->source_rfc1001_name[i] = toupper(nodename[i]);
> >
> > -       vol->source_rfc1001_name[15] = 0;
> > +       vol->source_rfc1001_name[RFC1001_NAME_LEN] = 0;
> >        /* null target name indicates to use *SMBSERVR default called name
> >           if we end up sending RFC1001 session initialize */
> >        vol->target_rfc1001_name[0] = 0;
> > @@ -1165,22 +1165,22 @@ cifs_parse_mount_options(char *options, const char *devname,
> >                        if (!value || !*value || (*value == ' ')) {
> >                                cFYI(1, "invalid (empty) netbiosname");
> >                        } else {
> > -                               memset(vol->source_rfc1001_name, 0x20, 15);
> > -                               for (i = 0; i < 15; i++) {
> > -                               /* BB are there cases in which a comma can be
> > -                               valid in this workstation netbios name (and need
> > -                               special handling)? */
> > -
> > -                               /* We do not uppercase netbiosname for user */
> > +                               memset(vol->source_rfc1001_name, 0x20,
> > +                                       RFC1001_NAME_LEN);
> > +                               /*
> > +                                * FIXME: are there cases in which a comma can
> > +                                * be valid in workstation netbios name (and
> > +                                * need special handling)?
> > +                                */
> > +                               for (i = 0; i < RFC1001_NAME_LEN; i++) {
> > +                                       /* don't ucase netbiosname for user */
> >                                        if (value[i] == 0)
> >                                                break;
> > -                                       else
> > -                                               vol->source_rfc1001_name[i] =
> > -                                                               value[i];
> > +                                       vol->source_rfc1001_name[i] = value[i];
> >                                }
> >                                /* The string has 16th byte zero still from
> >                                set at top of the function  */
> > -                               if ((i == 15) && (value[i] != 0))
> > +                               if (i == RFC1001_NAME_LEN && value[i] != 0)
> >                                        printk(KERN_WARNING "CIFS: netbiosname"
> >                                                " longer than 15 truncated.\n");
> >                        }
> > @@ -1190,7 +1190,8 @@ cifs_parse_mount_options(char *options, const char *devname,
> >                                cFYI(1, "empty server netbiosname specified");
> >                        } else {
> >                                /* last byte, type, is 0x20 for servr type */
> > -                               memset(vol->target_rfc1001_name, 0x20, 16);
> > +                               memset(vol->target_rfc1001_name, 0x20,
> > +                                       RFC1001_NAME_LEN_WITH_NULL);
> >
> >                                for (i = 0; i < 15; i++) {
> >                                /* BB are there cases in which a comma can be
> > @@ -1207,7 +1208,7 @@ cifs_parse_mount_options(char *options, const char *devname,
> >                                }
> >                                /* The string has 16th byte zero still from
> >                                   set at top of the function  */
> > -                               if ((i == 15) && (value[i] != 0))
> > +                               if (i == RFC1001_NAME_LEN && value[i] != 0)
> >                                        printk(KERN_WARNING "CIFS: server net"
> >                                        "biosname longer than 15 truncated.\n");
> >                        }
> > --
> > 1.7.3.2
> >
> >
> 
> Jeff, I do not see where RFC1001_NAME_LEN_WITH_NULL and RFC1000_NAME_LEN
> are defined.

In cifsglob.h...

There's a tool for this sort of thing called "cscope":

    http://cscope.sourceforge.net/

Cheers,
-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH] cifs: replace some hardcoded values with preprocessor constants

[Shirish Pargaonkar]

On Wed, Dec 1, 2010 at 9:07 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Wed, 1 Dec 2010 09:03:32 -0600
> Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> On Wed, Dec 1, 2010 at 8:49 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> > A number of places that deal with RFC1001/1002 negotiations have bare
>> > "15" or "16" values. Replace them with RFC_1001_NAME_LEN and
>> > RFC_1001_NAME_LEN_WITH_NULL.
>> >
>> > The patch also cleans up some checkpatch warnings for code surrounding
>> > the changes. This should apply cleanly on top of the patch to remove
>> > Local_System_Name.
>> >
>> > Reported-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> > Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> > ---
>> >  fs/cifs/connect.c |   37 +++++++++++++++++++------------------
>> >  1 files changed, 19 insertions(+), 18 deletions(-)
>> >
>> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> > index 5d48e09..a96324b 100644
>> > --- a/fs/cifs/connect.c
>> > +++ b/fs/cifs/connect.c
>> > @@ -64,8 +64,8 @@ struct smb_vol {
>> >        char *UNC;
>> >        char *UNCip;
>> >        char *iocharset;  /* local code page for mapping to and from Unicode */
>> > -       char source_rfc1001_name[16]; /* netbios name of client */
>> > -       char target_rfc1001_name[16]; /* netbios name of server for Win9x/ME */
>> > +       char source_rfc1001_name[RFC1001_NAME_LEN_WITH_NULL]; /* clnt nb name */
>> > +       char target_rfc1001_name[RFC1001_NAME_LEN_WITH_NULL]; /* srvr nb name */
>> >        uid_t cred_uid;
>> >        uid_t linux_uid;
>> >        gid_t linux_gid;
>> > @@ -816,11 +816,11 @@ cifs_parse_mount_options(char *options, const char *devname,
>> >         * informational, only used for servers that do not support
>> >         * port 445 and it can be overridden at mount time
>> >         */
>> > -       memset(vol->source_rfc1001_name, 0x20, 15);
>> > -       for (i = 0; i < strnlen(nodename, 15); i++)
>> > +       memset(vol->source_rfc1001_name, 0x20, RFC1001_NAME_LEN);
>> > +       for (i = 0; i < strnlen(nodename, RFC1001_NAME_LEN); i++)
>> >                vol->source_rfc1001_name[i] = toupper(nodename[i]);
>> >
>> > -       vol->source_rfc1001_name[15] = 0;
>> > +       vol->source_rfc1001_name[RFC1001_NAME_LEN] = 0;
>> >        /* null target name indicates to use *SMBSERVR default called name
>> >           if we end up sending RFC1001 session initialize */
>> >        vol->target_rfc1001_name[0] = 0;
>> > @@ -1165,22 +1165,22 @@ cifs_parse_mount_options(char *options, const char *devname,
>> >                        if (!value || !*value || (*value == ' ')) {
>> >                                cFYI(1, "invalid (empty) netbiosname");
>> >                        } else {
>> > -                               memset(vol->source_rfc1001_name, 0x20, 15);
>> > -                               for (i = 0; i < 15; i++) {
>> > -                               /* BB are there cases in which a comma can be
>> > -                               valid in this workstation netbios name (and need
>> > -                               special handling)? */
>> > -
>> > -                               /* We do not uppercase netbiosname for user */
>> > +                               memset(vol->source_rfc1001_name, 0x20,
>> > +                                       RFC1001_NAME_LEN);
>> > +                               /*
>> > +                                * FIXME: are there cases in which a comma can
>> > +                                * be valid in workstation netbios name (and
>> > +                                * need special handling)?
>> > +                                */
>> > +                               for (i = 0; i < RFC1001_NAME_LEN; i++) {
>> > +                                       /* don't ucase netbiosname for user */
>> >                                        if (value[i] == 0)
>> >                                                break;
>> > -                                       else
>> > -                                               vol->source_rfc1001_name[i] =
>> > -                                                               value[i];
>> > +                                       vol->source_rfc1001_name[i] = value[i];
>> >                                }
>> >                                /* The string has 16th byte zero still from
>> >                                set at top of the function  */
>> > -                               if ((i == 15) && (value[i] != 0))
>> > +                               if (i == RFC1001_NAME_LEN && value[i] != 0)
>> >                                        printk(KERN_WARNING "CIFS: netbiosname"
>> >                                                " longer than 15 truncated.\n");
>> >                        }
>> > @@ -1190,7 +1190,8 @@ cifs_parse_mount_options(char *options, const char *devname,
>> >                                cFYI(1, "empty server netbiosname specified");
>> >                        } else {
>> >                                /* last byte, type, is 0x20 for servr type */
>> > -                               memset(vol->target_rfc1001_name, 0x20, 16);
>> > +                               memset(vol->target_rfc1001_name, 0x20,
>> > +                                       RFC1001_NAME_LEN_WITH_NULL);
>> >
>> >                                for (i = 0; i < 15; i++) {
>> >                                /* BB are there cases in which a comma can be
>> > @@ -1207,7 +1208,7 @@ cifs_parse_mount_options(char *options, const char *devname,
>> >                                }
>> >                                /* The string has 16th byte zero still from
>> >                                   set at top of the function  */
>> > -                               if ((i == 15) && (value[i] != 0))
>> > +                               if (i == RFC1001_NAME_LEN && value[i] != 0)
>> >                                        printk(KERN_WARNING "CIFS: server net"
>> >                                        "biosname longer than 15 truncated.\n");
>> >                        }
>> > --
>> > 1.7.3.2
>> >
>> >
>>
>> Jeff, I do not see where RFC1001_NAME_LEN_WITH_NULL and RFC1000_NAME_LEN
>> are defined.
>
> In cifsglob.h...
>
> There's a tool for this sort of thing called "cscope":
>
>    http://cscope.sourceforge.net/
>
> Cheers,
> --
> Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>

Oh, I did not realize that these defines existed already.

Reviewed-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>