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)
> That being said, I don't feel like arguing strongly about this because
> ultimately I think this is a matter of style.
>
> I'll let those who have the powers to decide. :-)
Always a good strategy ;-)
Cheers,
Manuel.
