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)

> 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.

Reply via email to