diff mbox

[linux-cifs-client,05/12] cifs: rename cifs_strlcpy_to_host and make it use new functions

Message ID 1241036694-19940-6-git-send-email-jlayton@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jeff Layton April 29, 2009, 8:24 p.m. UTC
Rename cifs_strlcpy_to_host to cifs_strndup since that better describes
what this function really does. Then, convert it to use the new string
conversion and measurement functions that work in units of bytes rather
than wide chars.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/cifs/cifs_unicode.c |   41 +++++++++++++++++++++++++++++++++++++++++
 fs/cifs/cifs_unicode.h |    2 ++
 fs/cifs/cifssmb.c      |   48 +++++++-----------------------------------------
 3 files changed, 50 insertions(+), 41 deletions(-)

Comments

Suresh Jayaraman April 30, 2009, 9:05 a.m. UTC | #1
Jeff Layton wrote:
> Rename cifs_strlcpy_to_host to cifs_strndup since that better describes
> what this function really does. Then, convert it to use the new string
> conversion and measurement functions that work in units of bytes rather
> than wide chars.
> 
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
>  fs/cifs/cifs_unicode.c |   41 +++++++++++++++++++++++++++++++++++++++++
>  fs/cifs/cifs_unicode.h |    2 ++
>  fs/cifs/cifssmb.c      |   48 +++++++-----------------------------------------
>  3 files changed, 50 insertions(+), 41 deletions(-)
> 
> diff --git a/fs/cifs/cifs_unicode.c b/fs/cifs/cifs_unicode.c
> index cd938bd..356f00c 100644
> --- a/fs/cifs/cifs_unicode.c
> +++ b/fs/cifs/cifs_unicode.c
> @@ -245,3 +245,44 @@ cifs_strtoUCS(__le16 *to, const char *from, int len,
>  	return i;
>  }
>  
> +/*
> + * cifs_strndup - copy a string from wire format to the local codepage
> + * @dst - pointer to pointer of destination string
> + * @src - source string
> + * @maxlen - don't walk past this many bytes in the source string
> + * @is_unicode - is this a unicode string?
> + * @codepage - destination codepage
> + *
> + * Take a string given by the server, convert it to the local codepage and
> + * put it in a new buffer. Returns length of the new buffer in bytes or a
> + * negative error code. A pointer to the new buffer is placed into *dst.
> + */
> +int
> +cifs_strndup(char **dst, const char *src, const int maxlen,
> +	     const bool is_unicode, const struct nls_table *codepage)
> +{
> +	int len;
> +
> +	if (is_unicode) {
> +		len = cifs_ucs2_bytes((__le16 *) src, maxlen, codepage);
> +		len += nls_nullsize(codepage);
> +		*dst = kmalloc(len, GFP_KERNEL);
> +		if (!*dst)
> +			goto err_exit;
> +		cifs_from_ucs2(*dst, (__le16 *) src, len, maxlen, codepage,
> +			       false);
> +	} else {
> +		len = strnlen(src, maxlen);
> +		len++;
> +		*dst = kmalloc(len, GFP_KERNEL);
> +		if (!*dst)
> +			goto err_exit;
> +		strlcpy(*dst, src, len);
> +	}
> +	return len;

Do we really need to return len? Does any of the callers make use of
this, now?


> +
> +err_exit:
> +	cERROR(1, ("Failed to allocate buffer for string\n"));
> +	return -ENOMEM;
> +}
> +
> diff --git a/fs/cifs/cifs_unicode.h b/fs/cifs/cifs_unicode.h
> index 1857f5f..9c7c9fc 100644
> --- a/fs/cifs/cifs_unicode.h
> +++ b/fs/cifs/cifs_unicode.h
> @@ -78,6 +78,8 @@ int cifs_ucs2_bytes(const __le16 *from, int maxbytes,
>  		    const struct nls_table *codepage);
>  int cifs_strfromUCS_le(char *, const __le16 *, int, const struct nls_table *);
>  int cifs_strtoUCS(__le16 *, const char *, int, const struct nls_table *);
> +int cifs_strndup(char **dst, const char *src, const int maxlen,
> +		 const bool is_unicode, const struct nls_table *codepage);
>  #endif
>  
>  /*
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index cadacae..be90f22 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -81,41 +81,6 @@ static struct {
>  #endif /* CONFIG_CIFS_WEAK_PW_HASH */
>  #endif /* CIFS_POSIX */
>  
> -/* Allocates buffer into dst and copies smb string from src to it.
> - * caller is responsible for freeing dst if function returned 0.
> - * returns:
> - * 	on success - 0
> - *	on failure - errno
> - */
> -static int
> -cifs_strlcpy_to_host(char **dst, const char *src, const int maxlen,
> -		 const bool is_unicode, const struct nls_table *nls_codepage)
> -{
> -	int plen;
> -
> -	if (is_unicode) {
> -		plen = UniStrnlen((wchar_t *)src, maxlen);
> -		*dst = kmalloc((4 * plen) + 2, GFP_KERNEL);
> -		if (!*dst)
> -			goto cifs_strlcpy_to_host_ErrExit;
> -		cifs_strfromUCS_le(*dst, (__le16 *)src, plen, nls_codepage);
> -		(*dst)[plen] = 0;
> -		(*dst)[plen+1] = 0; /* needed for Unicode */
> -	} else {
> -		plen = strnlen(src, maxlen);
> -		*dst = kmalloc(plen + 2, GFP_KERNEL);
> -		if (!*dst)
> -			goto cifs_strlcpy_to_host_ErrExit;
> -		strlcpy(*dst, src, plen);
> -	}
> -	return 0;
> -
> -cifs_strlcpy_to_host_ErrExit:
> -	cERROR(1, ("Failed to allocate buffer for string\n"));
> -	return -ENOMEM;
> -}
> -
> -
>  /* Mark as invalid, all open files on tree connections since they
>     were closed when session to server was lost */
>  static void mark_open_files_invalid(struct cifsTconInfo *pTcon)
> @@ -4008,19 +3973,20 @@ parse_DFS_referrals(TRANSACTION2_GET_DFS_REFER_RSP *pSMBr,
>  		/* copy DfsPath */
>  		temp = (char *)ref + le16_to_cpu(ref->DfsPathOffset);
>  		max_len = data_end - temp;
> -		rc = cifs_strlcpy_to_host(&(node->path_name), temp,
> -					max_len, is_unicode, nls_codepage);
> -		if (rc)
> +		rc = cifs_strndup(&node->path_name, temp, max_len,
> +				  is_unicode, nls_codepage);
> +		if (rc < 0)
>  			goto parse_DFS_referrals_exit;
>  
>  		/* copy link target UNC */
>  		temp = (char *)ref + le16_to_cpu(ref->NetworkAddressOffset);
>  		max_len = data_end - temp;
> -		rc = cifs_strlcpy_to_host(&(node->node_name), temp,
> -					max_len, is_unicode, nls_codepage);
> -		if (rc)
> +		rc = cifs_strndup(&node->node_name, temp, max_len,
> +				  is_unicode, nls_codepage);
> +		if (rc < 0)
>  			goto parse_DFS_referrals_exit;
>  
> +		rc = 0;
>  		ref += le16_to_cpu(ref->Size);
>  	}
>
Jeff Layton April 30, 2009, 10:40 a.m. UTC | #2
On Thu, 30 Apr 2009 14:35:15 +0530
Suresh Jayaraman <sjayaraman@suse.de> wrote:

> Jeff Layton wrote:
> > Rename cifs_strlcpy_to_host to cifs_strndup since that better describes
> > what this function really does. Then, convert it to use the new string
> > conversion and measurement functions that work in units of bytes rather
> > than wide chars.
> > 
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> >  fs/cifs/cifs_unicode.c |   41 +++++++++++++++++++++++++++++++++++++++++
> >  fs/cifs/cifs_unicode.h |    2 ++
> >  fs/cifs/cifssmb.c      |   48 +++++++-----------------------------------------
> >  3 files changed, 50 insertions(+), 41 deletions(-)
> > 
> > diff --git a/fs/cifs/cifs_unicode.c b/fs/cifs/cifs_unicode.c
> > index cd938bd..356f00c 100644
> > --- a/fs/cifs/cifs_unicode.c
> > +++ b/fs/cifs/cifs_unicode.c
> > @@ -245,3 +245,44 @@ cifs_strtoUCS(__le16 *to, const char *from, int len,
> >  	return i;
> >  }
> >  
> > +/*
> > + * cifs_strndup - copy a string from wire format to the local codepage
> > + * @dst - pointer to pointer of destination string
> > + * @src - source string
> > + * @maxlen - don't walk past this many bytes in the source string
> > + * @is_unicode - is this a unicode string?
> > + * @codepage - destination codepage
> > + *
> > + * Take a string given by the server, convert it to the local codepage and
> > + * put it in a new buffer. Returns length of the new buffer in bytes or a
> > + * negative error code. A pointer to the new buffer is placed into *dst.
> > + */
> > +int
> > +cifs_strndup(char **dst, const char *src, const int maxlen,
> > +	     const bool is_unicode, const struct nls_table *codepage)
> > +{
> > +	int len;
> > +
> > +	if (is_unicode) {
> > +		len = cifs_ucs2_bytes((__le16 *) src, maxlen, codepage);
> > +		len += nls_nullsize(codepage);
> > +		*dst = kmalloc(len, GFP_KERNEL);
> > +		if (!*dst)
> > +			goto err_exit;
> > +		cifs_from_ucs2(*dst, (__le16 *) src, len, maxlen, codepage,
> > +			       false);
> > +	} else {
> > +		len = strnlen(src, maxlen);
> > +		len++;
> > +		*dst = kmalloc(len, GFP_KERNEL);
> > +		if (!*dst)
> > +			goto err_exit;
> > +		strlcpy(*dst, src, len);
> > +	}
> > +	return len;
> 
> Do we really need to return len? Does any of the callers make use of
> this, now?
> 

We could just as easily make this function return a char pointer
instead and use PTR_ERR() to return errors. I had an earlier set that
did that and changed it at one point since I thought I needed the
length in one of the callers. It turns out that I don't. Returning a
char pointer here is probably a cleaner interface, so I'll make
that change in a bit.

> 
> > +
> > +err_exit:
> > +	cERROR(1, ("Failed to allocate buffer for string\n"));
> > +	return -ENOMEM;
> > +}
> > +
> > diff --git a/fs/cifs/cifs_unicode.h b/fs/cifs/cifs_unicode.h
> > index 1857f5f..9c7c9fc 100644
> > --- a/fs/cifs/cifs_unicode.h
> > +++ b/fs/cifs/cifs_unicode.h
> > @@ -78,6 +78,8 @@ int cifs_ucs2_bytes(const __le16 *from, int maxbytes,
> >  		    const struct nls_table *codepage);
> >  int cifs_strfromUCS_le(char *, const __le16 *, int, const struct nls_table *);
> >  int cifs_strtoUCS(__le16 *, const char *, int, const struct nls_table *);
> > +int cifs_strndup(char **dst, const char *src, const int maxlen,
> > +		 const bool is_unicode, const struct nls_table *codepage);
> >  #endif
> >  
> >  /*
> > diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> > index cadacae..be90f22 100644
> > --- a/fs/cifs/cifssmb.c
> > +++ b/fs/cifs/cifssmb.c
> > @@ -81,41 +81,6 @@ static struct {
> >  #endif /* CONFIG_CIFS_WEAK_PW_HASH */
> >  #endif /* CIFS_POSIX */
> >  
> > -/* Allocates buffer into dst and copies smb string from src to it.
> > - * caller is responsible for freeing dst if function returned 0.
> > - * returns:
> > - * 	on success - 0
> > - *	on failure - errno
> > - */
> > -static int
> > -cifs_strlcpy_to_host(char **dst, const char *src, const int maxlen,
> > -		 const bool is_unicode, const struct nls_table *nls_codepage)
> > -{
> > -	int plen;
> > -
> > -	if (is_unicode) {
> > -		plen = UniStrnlen((wchar_t *)src, maxlen);
> > -		*dst = kmalloc((4 * plen) + 2, GFP_KERNEL);
> > -		if (!*dst)
> > -			goto cifs_strlcpy_to_host_ErrExit;
> > -		cifs_strfromUCS_le(*dst, (__le16 *)src, plen, nls_codepage);
> > -		(*dst)[plen] = 0;
> > -		(*dst)[plen+1] = 0; /* needed for Unicode */
> > -	} else {
> > -		plen = strnlen(src, maxlen);
> > -		*dst = kmalloc(plen + 2, GFP_KERNEL);
> > -		if (!*dst)
> > -			goto cifs_strlcpy_to_host_ErrExit;
> > -		strlcpy(*dst, src, plen);
> > -	}
> > -	return 0;
> > -
> > -cifs_strlcpy_to_host_ErrExit:
> > -	cERROR(1, ("Failed to allocate buffer for string\n"));
> > -	return -ENOMEM;
> > -}
> > -
> > -
> >  /* Mark as invalid, all open files on tree connections since they
> >     were closed when session to server was lost */
> >  static void mark_open_files_invalid(struct cifsTconInfo *pTcon)
> > @@ -4008,19 +3973,20 @@ parse_DFS_referrals(TRANSACTION2_GET_DFS_REFER_RSP *pSMBr,
> >  		/* copy DfsPath */
> >  		temp = (char *)ref + le16_to_cpu(ref->DfsPathOffset);
> >  		max_len = data_end - temp;
> > -		rc = cifs_strlcpy_to_host(&(node->path_name), temp,
> > -					max_len, is_unicode, nls_codepage);
> > -		if (rc)
> > +		rc = cifs_strndup(&node->path_name, temp, max_len,
> > +				  is_unicode, nls_codepage);
> > +		if (rc < 0)
> >  			goto parse_DFS_referrals_exit;
> >  
> >  		/* copy link target UNC */
> >  		temp = (char *)ref + le16_to_cpu(ref->NetworkAddressOffset);
> >  		max_len = data_end - temp;
> > -		rc = cifs_strlcpy_to_host(&(node->node_name), temp,
> > -					max_len, is_unicode, nls_codepage);
> > -		if (rc)
> > +		rc = cifs_strndup(&node->node_name, temp, max_len,
> > +				  is_unicode, nls_codepage);
> > +		if (rc < 0)
> >  			goto parse_DFS_referrals_exit;
> >  
> > +		rc = 0;
> >  		ref += le16_to_cpu(ref->Size);
> >  	}
> >  
> 
> 
> -- 
> Suresh Jayaraman
diff mbox

Patch

diff --git a/fs/cifs/cifs_unicode.c b/fs/cifs/cifs_unicode.c
index cd938bd..356f00c 100644
--- a/fs/cifs/cifs_unicode.c
+++ b/fs/cifs/cifs_unicode.c
@@ -245,3 +245,44 @@  cifs_strtoUCS(__le16 *to, const char *from, int len,
 	return i;
 }
 
+/*
+ * cifs_strndup - copy a string from wire format to the local codepage
+ * @dst - pointer to pointer of destination string
+ * @src - source string
+ * @maxlen - don't walk past this many bytes in the source string
+ * @is_unicode - is this a unicode string?
+ * @codepage - destination codepage
+ *
+ * Take a string given by the server, convert it to the local codepage and
+ * put it in a new buffer. Returns length of the new buffer in bytes or a
+ * negative error code. A pointer to the new buffer is placed into *dst.
+ */
+int
+cifs_strndup(char **dst, const char *src, const int maxlen,
+	     const bool is_unicode, const struct nls_table *codepage)
+{
+	int len;
+
+	if (is_unicode) {
+		len = cifs_ucs2_bytes((__le16 *) src, maxlen, codepage);
+		len += nls_nullsize(codepage);
+		*dst = kmalloc(len, GFP_KERNEL);
+		if (!*dst)
+			goto err_exit;
+		cifs_from_ucs2(*dst, (__le16 *) src, len, maxlen, codepage,
+			       false);
+	} else {
+		len = strnlen(src, maxlen);
+		len++;
+		*dst = kmalloc(len, GFP_KERNEL);
+		if (!*dst)
+			goto err_exit;
+		strlcpy(*dst, src, len);
+	}
+	return len;
+
+err_exit:
+	cERROR(1, ("Failed to allocate buffer for string\n"));
+	return -ENOMEM;
+}
+
diff --git a/fs/cifs/cifs_unicode.h b/fs/cifs/cifs_unicode.h
index 1857f5f..9c7c9fc 100644
--- a/fs/cifs/cifs_unicode.h
+++ b/fs/cifs/cifs_unicode.h
@@ -78,6 +78,8 @@  int cifs_ucs2_bytes(const __le16 *from, int maxbytes,
 		    const struct nls_table *codepage);
 int cifs_strfromUCS_le(char *, const __le16 *, int, const struct nls_table *);
 int cifs_strtoUCS(__le16 *, const char *, int, const struct nls_table *);
+int cifs_strndup(char **dst, const char *src, const int maxlen,
+		 const bool is_unicode, const struct nls_table *codepage);
 #endif
 
 /*
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index cadacae..be90f22 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -81,41 +81,6 @@  static struct {
 #endif /* CONFIG_CIFS_WEAK_PW_HASH */
 #endif /* CIFS_POSIX */
 
-/* Allocates buffer into dst and copies smb string from src to it.
- * caller is responsible for freeing dst if function returned 0.
- * returns:
- * 	on success - 0
- *	on failure - errno
- */
-static int
-cifs_strlcpy_to_host(char **dst, const char *src, const int maxlen,
-		 const bool is_unicode, const struct nls_table *nls_codepage)
-{
-	int plen;
-
-	if (is_unicode) {
-		plen = UniStrnlen((wchar_t *)src, maxlen);
-		*dst = kmalloc((4 * plen) + 2, GFP_KERNEL);
-		if (!*dst)
-			goto cifs_strlcpy_to_host_ErrExit;
-		cifs_strfromUCS_le(*dst, (__le16 *)src, plen, nls_codepage);
-		(*dst)[plen] = 0;
-		(*dst)[plen+1] = 0; /* needed for Unicode */
-	} else {
-		plen = strnlen(src, maxlen);
-		*dst = kmalloc(plen + 2, GFP_KERNEL);
-		if (!*dst)
-			goto cifs_strlcpy_to_host_ErrExit;
-		strlcpy(*dst, src, plen);
-	}
-	return 0;
-
-cifs_strlcpy_to_host_ErrExit:
-	cERROR(1, ("Failed to allocate buffer for string\n"));
-	return -ENOMEM;
-}
-
-
 /* Mark as invalid, all open files on tree connections since they
    were closed when session to server was lost */
 static void mark_open_files_invalid(struct cifsTconInfo *pTcon)
@@ -4008,19 +3973,20 @@  parse_DFS_referrals(TRANSACTION2_GET_DFS_REFER_RSP *pSMBr,
 		/* copy DfsPath */
 		temp = (char *)ref + le16_to_cpu(ref->DfsPathOffset);
 		max_len = data_end - temp;
-		rc = cifs_strlcpy_to_host(&(node->path_name), temp,
-					max_len, is_unicode, nls_codepage);
-		if (rc)
+		rc = cifs_strndup(&node->path_name, temp, max_len,
+				  is_unicode, nls_codepage);
+		if (rc < 0)
 			goto parse_DFS_referrals_exit;
 
 		/* copy link target UNC */
 		temp = (char *)ref + le16_to_cpu(ref->NetworkAddressOffset);
 		max_len = data_end - temp;
-		rc = cifs_strlcpy_to_host(&(node->node_name), temp,
-					max_len, is_unicode, nls_codepage);
-		if (rc)
+		rc = cifs_strndup(&node->node_name, temp, max_len,
+				  is_unicode, nls_codepage);
+		if (rc < 0)
 			goto parse_DFS_referrals_exit;
 
+		rc = 0;
 		ref += le16_to_cpu(ref->Size);
 	}