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
>
