On 26 April 2012 20:56, Dodji Seketeli <do...@seketeli.org> wrote:
> Manuel López-Ibáñez <lopeziba...@gmail.com> a écrit:
>
>> On 26 April 2012 20:11, Dodji Seketeli <do...@seketeli.org> wrote:
>>> Manuel López-Ibáñez <lopeziba...@gmail.com> 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. :-)
Oh, no apologies needed. Thanks you chimed in! I wish more people were
working on this stuff! Anyway, let''s wait to see what the decision
makers say.
Cheers,
Manuel.
