[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1482282 - svn_nls_init on Windows

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Tue, 14 May 2013 18:18:58 +0100 (BST)

Bert Huijben wrote:

> Julian Foad wrote:
>> (1) Inserting a UCS-2 string into a UTF-8 error message will surely fail
>> in one way or another.
>>
>> (2) Just below this error handling, the code to write the null terminator
>> is bogus.
>>
>> I suggest:
[...]
>> -          err = svn_error_createf(apr_err, NULL,
>> -                                  _("Can't convert module path "
>> -                                    "to UTF-8 from UCS-2: '%s'"),
>> -                                  ucs2_path);
>> +          err = svn_error_create(apr_err, NULL,
>> +                                 _("Can't convert module path "
>> +                                   "to UTF-8 from UCS-2");
>
> apr_err is no longer defined, so this code wouldn't compile.

Sure, that's just because this pseudo-patch is against the older code, not your latest version.

> Most likely the second by of the UCS-2 string is 0 and in all cases the
> final word is 0, so this won't crash. So we can fix this in a later release
> if necessary.

I agree.

>>          }
>>        else
>>          {
>> -          utf8_path[outlength - outbytes] = '\0';
>> +          utf8_path[outbytes] = '\0';
>>            internal_path = svn_dirent_internal_style(utf8_path, pool);
>
> I'm surprised that this code worked correctly in 1.7... but it did. This
> code path has been active in at least the SlikSvn build.

For some definition of 'correctly' :-)

- Julian

>
>     Bert
>>
>> [...]
>>
>> ]]]
>>
>> The existing code puts the null terminator in the wrong place.  This will
>> truncate any path that is longer than PATH_MAX/2, and will fail to terminate
>> any path shorter than PATH_MAX/2.  WideCharToMultiByte() explicitly does
>> not promise to write a null terminator.  This code will work despite this
>> bug iff:
>>
>>   * path length < MAX_PATH/2
>>
>> and either of:
>>
>>   * WideCharToMultiByte() adds a null terminator
>>   * apr_alloc() clears the memory
>>
>> Otherwise, it will produce the wrong result.
>>
>> - Julian
>
Received on 2013-05-14 19:19:52 CEST

This is an archived mail posted to the Subversion Dev mailing list.