Re: [PATCH 05/12] cifs: rename cifs_strlcpy_to_host and make it use new functions

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 <[email protected]>
> ---
>  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);
>  	}
>  


-- 
Suresh Jayaraman