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

On Wed, Apr 29, 2009 at 2:32 PM, Jeff Layton <[email protected]> wrote:
> On Wed, 29 Apr 2009 12:47:00 -0500
> Shirish Pargaonkar <[email protected]> wrote:
>
>> On Wed, Apr 29, 2009 at 12:25 PM, Jeff Layton <[email protected]> wrote:
>> > Rename cifs_strlcpy_to_host to cifs_strldup_to_host 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 |    3 +++
>> >  fs/cifs/cifssmb.c      |   48 +++++++-----------------------------------------
>> >  3 files changed, 51 insertions(+), 41 deletions(-)
>> >
>> > diff --git a/fs/cifs/cifs_unicode.c b/fs/cifs/cifs_unicode.c
>> > index 29db445..e325d3d 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_strldup_to_host - 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, covert 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_strldup_to_host(char **dst, const char *src, const int maxlen,
>> > +                    const bool is_unicode, const struct nls_table *codepage)
>>
>> While translating and copying the string received on the wire to the
>> string in the local charset,
>> does cifs not have to worrry about mapchars (value of remap) mount
>> option since cifs_from_ucs2le
>> does use mapchar variable or it does not matter so mapchars value
>> while calling cifs_from_ucs2le
>> be always false?
>>
>
> The strings that this function copies are not always (and in fact, are
> seldom) paths.
>
> The existing code outside of the readdir codepath doesn't care, so I'm
> not planning to worry about it here. That can be fixed later if there's
> a need for it.
>
>> > +{
>> > +       int len;
>> > +
>> > +       if (is_unicode) {
>> > +               len = cifs_ucs2le_bytes((__le16 *) src, maxlen, codepage);
>> > +               len += nls_nullsize(codepage);
>> > +               *dst = kmalloc(len, GFP_KERNEL);
>> > +               if (!*dst)
>> > +                       goto err_exit;
>> > +               cifs_from_ucs2le(*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 ef7b907..2ec6375 100644
>> > --- a/fs/cifs/cifs_unicode.h
>> > +++ b/fs/cifs/cifs_unicode.h
>> > @@ -78,6 +78,9 @@ int cifs_from_ucs2le(char *to, const __le16 *from, int tolen, int fromlen,
>> >                     const struct nls_table *codepage, bool mapchar);
>> >  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_strldup_to_host(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 33e433b..17c4ba9 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_strldup_to_host(&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_strldup_to_host(&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);
>> >        }
>> >
>> > --
>> > 1.6.0.6
>> >
>> > _______________________________________________
>> > linux-cifs-client mailing list
>> > [email protected]
>> > https://lists.samba.org/mailman/listinfo/linux-cifs-client
>> >
>
>
> --
> Jeff Layton <[email protected]>
>

Jeff,

OK.  Let me look at this more, the reason I was concerned was I
remebered samba bugzilla bug 4277.

Regards,

Shirish