Re: gearmand - PATH_MAX patch review

["Philipp A. Hartmann" <ph@sorgh.de>]

Pino,

On 18/06/12 13:04, Pino Toscano wrote:
> 
> Alle lunedì 18 giugno 2012, Philipp A. Hartmann ha scritto:
> 
>> Alternative sketched below, untested of course.
>>
>> On 18/06/12 09:50, Samuel Thibault wrote:
>>> Barry deFreese, le Mon 18 Jun 2012 00:20:17 -0400, a écrit :
>>>> +#ifdef _GNU_SOURCE
>>>
>>> _GNU_SOURCE is not something you can test. It's the application
>>> which is supposed to define it. Test __GLIBC__ instead
>>>
>>>> +        char *getcwd_buf= get_current_dir_name();
>>
>>             std::string const getcwd_str = get_current_dir_name();
> 
> get_current_dir_name() returns a new char* buffer which must be freed 
> with free(), so your instruction would leak that memory.

Doh, you're right of course.  Good catch.  My intent was to immediately
free() the allocated buffer after copying it into the string.

So this would the look like:

  char const *      getcwd_buf = get_current_dir_name();
  std::string const getcwd_str = getcwd_buf;

  free( getcwd_buf );
  getcwd_buf = getcwd_str.c_str();

> A better solution would be something like this:
>
>   class Buffer
[snip]
>   Buffer b(get_current_dir_name());
>   // use b as if it were a char*

I agree that such kind of smart-pointer would be a better solution in
general.  It may not be justified for this particular platform-specific
fix, though.  Maybe there is an appropriate helper already present in
upstream's source, which should then be reused.

Thanks for pointing this out,
 Philipp