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.