Re: [PATCH] cifs: Fix insufficient memory allocation for nativeFileSystem field

Jeff Layton wrote:
> On Tue, 07 Apr 2009 18:45:46 +0530
> Suresh Jayaraman <[email protected]> wrote:
> 
>> Do we still need this conversion again?
>>
> 
> I know this isn't a "real" patch submission yet, but some comments
> below...

Indeed, it's quick, hackish and intended just to know where we are headed..
I appreciate the review much, nevertheless.

>> diff --git a/fs/cifs/cifs_unicode.h b/fs/cifs/cifs_unicode.h
>> index 14eb9a2..0396bdc 100644
>> --- a/fs/cifs/cifs_unicode.h
>> +++ b/fs/cifs/cifs_unicode.h
>> @@ -159,6 +159,23 @@ UniStrnlen(const wchar_t *ucs1, int maxlen)
>>  }
>>
>>  /*
>> + * UniStrnlenBytes: Return the length in bytes of a UTF-8 string
>> + */
>> +static inline size_t
>> +UniStrnlenBytes(const unsigned char *str, int maxlen)
>> +{
>> +       size_t nbytes = 0;
>> +       wchar_t *uni;
> 		^^^^^
> I think you need to allocate actual storage for the character here.

yes.

>> +
>> +       while (*str++) {
>> +               /* convert each char, find its length and add to nbytes */
>> +               if (char2uni(str, maxlen, uni) > 0)
>> +                       nbytes += strnlen(uni, NLS_MAX_CHARSET_SIZE);
> 
> "uni" is a ptr to a wchar_t, but you're treating it as a string.
> There's no guarantee that it'll be null-terminated. I might be
> mistaken, but doesn't char2uni return the length of the converted
> character in bytes? Tallying up the return from those is probably
> the thing to do.

sorry stupid bugs, it's not a patch that qualifies for submission/review..
char2uni returns length in bytes. I did look at others some fs/* code
which sort of does this, didn't get it right.

I have made a different patch based on the recent comments and it's on
its way..

Thanks for the review and comments.

-- 
Suresh Jayaraman