Re: [PATCH 01/10] cifs: add function to get length of NULL termination in bytes

On Wed, Apr 29, 2009 at 9:56 AM, Jeff Layton <[email protected]> wrote:
> On Wed, 29 Apr 2009 09:39:58 -0500
> Shirish Pargaonkar <[email protected]> wrote:
>
>> On Wed, Apr 29, 2009 at 9:12 AM, Jeff Layton <[email protected]> wrote:
>> > On Wed, 29 Apr 2009 08:58:22 -0500
>> > Shirish Pargaonkar <[email protected]> wrote:
>> >
>> >> On Wed, Apr 29, 2009 at 8:29 AM, Jeff Layton <[email protected]> wrote:
>> >> > It's possible to have the null terminator for a charset be a single or
>> >> > multiple character. Add a function to tell us how long it should be.
>> >> >
>> >> > Signed-off-by: Jeff Layton <[email protected]>
>> >> > ---
>> >> >  fs/cifs/cifs_unicode.h |   19 +++++++++++++++++++
>> >> >  1 files changed, 19 insertions(+), 0 deletions(-)
>> >> >
>> >> > diff --git a/fs/cifs/cifs_unicode.h b/fs/cifs/cifs_unicode.h
>> >> > index 14eb9a2..6bffab5 100644
>> >> > --- a/fs/cifs/cifs_unicode.h
>> >> > +++ b/fs/cifs/cifs_unicode.h
>> >> > @@ -64,6 +64,25 @@ int cifs_strtoUCS(__le16 *, const char *, int, const struct nls_table *);
>> >> >  #endif
>> >> >
>> >> >  /*
>> >> > + * null_charlen - return length of null character for codepage
>> >> > + * @codepage - codepage for which to return length of NULL terminator
>> >> > + *
>> >> > + * Since we can't guarantee that the null terminator will be a particular
>> >> > + * length, we have to check against the codepage. If there's a problem
>> >> > + * determining it, assume a single-byte NULL terminator.
>> >> > + */
>> >> > +static inline int
>> >> > +null_charlen(const struct nls_table *codepage)
>> >> > +{
>> >> > +       int charlen;
>> >> > +       char tmp[NLS_MAX_CHARSET_SIZE];
>> >> > +
>> >> > +       charlen = codepage->uni2char(0, tmp, NLS_MAX_CHARSET_SIZE);
>> >> > +
>> >> > +       return charlen > 0 ? charlen : 1;
>> >> > +}
>> >> > +
>> >> > +/*
>> >> >  * UniStrcat:  Concatenate the second string to the first
>> >> >  *
>> >> >  * Returns:
>> >> > --
>> >> > 1.6.0.6
>> >> >
>> >> > _______________________________________________
>> >> > linux-cifs-client mailing list
>> >> > [email protected]
>> >> > https://lists.samba.org/mailman/listinfo/linux-cifs-client
>> >> >
>> >>
>> >> For some of the charsets I looked at under fs/nls, it looks like uni2char
>> >> always returns 1, I think to indicate the function succeeded as opposed
>> >> to sending an error.
>> >> Are there any charsets that you might have looked at whose
>> >> uni2char function returns more than 1 byte as size of the null character?
>> >
>> > No, I haven't see any, but I didn't do an exhaustive search. Given the
>> > number of problems we've had in this area, I'm leery of making any
>> > assumptions about these lengths. It's also possible that at some point
>> > in the future we could have an in-kernel version of UTF-16 or UTF-32.
>> > In the event of that we'll need to deal with multibyte null termination.
>> >
>> > So I think it makes sense to use a helper function for determining this
>> > rather than sprinkling "+1" to lengths all over the code. The overhead
>> > looks pretty minimal anyway.
>> >
>> > --
>> > Jeff Layton <[email protected]>
>> >
>>
>> Jeff, I do not think uni2char returns the lenght of a character is bytes.
>
> Yes, it does.

Alright but the reason I had doubt was, even char2uni returns 1, now that is
not correct when a code point from a charset is mapped/encoded to a mutlibyte
code value using UTF-16.

>
>> But what you are saying as I understand is, in the future null_charlen may
>> call some other function than uni2char to get the size of a null character
>> to get an accurate value, so interface will remain same, just the functionility
>> (of null_charlen) may change.
>
> Correct. We could (in principle) make that just return 1 for now, but
> then we'd have to worry about fixing that in the future if a multibyte
> null terminator were ever needed.
>
> --
> Jeff Layton <[email protected]>
>