On 2/8/2011 4:29 PM, Steve Hay wrote:
> William A. Rowe Jr. wrote on 2011-02-08:
>> On 2/8/2011 8:29 AM, Steve Hay wrote:
>>> +        int len = (int)strlen(newarr[arg]) + 1; +        *env =
>>> (char*)_malloc_dbg(len * sizeof(char), _CRT_BLOCK, __FILE__, __LINE__);
>>> +        strcpy_s(*env, len, newarr[arg]);
>>
>> That's just sick ... It's inexcusable to use strcpy when len is already
>> known :)
>>
>> Very interesting observations, are you in a position to try this with
>> the older
>> C compilers?  Interested to know if we can just apply this to all
>> flavors without
>> any headaches.
> 
> I'm not sure if strcpy_s() is as sick as strcpy() would be, given that it 
> takes a len argument anyway (?), but I've been looking at saving a little 
> work in the function concerned anyway, by copying the new environment strings 
> into place directly from the return value of the UTF-8 conversion function, 
> rather than going through the intermediate newarr[] array as my quick hack 
> currently does.

It is still testing for \0, which strlen above did.  Some people just like
copying arbitrary length strings into fixed length buffers, which strcpy_s()
was supposed to protect.  (Never mind that it's bugged, let's just have an
API which is safe from crashes).

> The function at the foot of this email is more along the lines of what I'm 
> thinking, and is still working correctly. Would this approach be an 
> improvement when suitably tidied up?

Looks great.  FWIW this really has been honed as an apr fix, perhaps we move
the discussion to dev@apr?  In the meantime, let me know how your tests go
so I can get to committing your proposed replacement.

Thanks!

Bill

Reply via email to