Manuel López-Ibáñez <[email protected]> a écrit:
> On 26 April 2012 20:11, Dodji Seketeli <[email protected]> wrote:
>> Manuel López-Ibáñez <[email protected]> a écrit:
>>
>>> Why not remove this comment and free file here with XDELETEVEC (file) ?
>>>
>>>> + canonical_path = maybe_shorter_path (path);
>>>> + if (canonical_path != NULL && canonical_path != path)
>>>> + {
>>>> + /* The canonical path was newly allocated. Let's free the
>>>> + non-canonical one. */
>>>> + free (path);
>>>> + path = canonical_path;
>>>> + }
>>>> +
>>>
>>> This way you avoid doing all this extra work here.
>>
>> If I follow my personal style, I'd prefer not having a function delete
>> what it receives in argument, unless the name of that function makes it
>> really obvious. Furthermore, that function could be later re-used on a
>> string that is not necessarily meant to be deleted.
>
> Fair enough. Still the comment at the top of the function needs to be changed:
>
> +/* Canonicalize the path to FILE. Return the canonical form if it is
> + shorter, otherwise return the original. This function may free the
> + memory pointed by FILE. */
>
> and then the function could return NULL when it fails to find a shorter path:
>
> + else
> + {
> + XDELETEVEC (file2);
> + return file;
> + }
> +}
>
> here. This way you can still simplify the caller by just checking if
> (canonical_path)
OK by me. Thank you for caring about this.
Would you mind just taking it over again and submit a proper patch +
ChangeLog? I just chimed in to help; I didn't mean to step on your
toes. :-)
Cheerio.
--
Dodji
