This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [libcpp] maybe canonicalize system paths in line-map


On 26 April 2012 20:11, Dodji Seketeli <dodji@seketeli.org> wrote:
> Manuel López-Ibáñez <lopezibanez@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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]