On 2012-07-20 13:09, Joel Rosdahl wrote:
Thanks. Two things:

1. The patch canonicalizes the "from" parameter, but I think that the
"to" parameter should be canonicalized as well, for two reasons: a)
realpath() expands symlinks, and comparing (see
common_dir_prefix_length) an expanded path with an unexpanded path
won't work well when symlinks are present. b) The "to" parameter
(which for instance may be a path given on the command line) could
also contain path elements that could benefit from canonicalization
(to increase cache hits).

2. The patch canonicalizes the "from" parameter every time
get_relative_path is called, which is unnecessary since it's always
the same. Therefore, as mentioned earlier, I think that its' better to
do the canonicalization in make_relative_path where
current_working_dir can be canonicalized when created.

Thanks for the comments. Please discard the previous patch. This attached version of the patch should be better. It canonicalizes the current working directory once and canonicalizes the path each time get_relative_path is called.

Thanks,
Eric

diff -uNr ccache-3.1.7/ccache.c ccache-3.1.7.patched/ccache.c
--- ccache-3.1.7/ccache.c       2012-01-08 09:40:55.000000000 -0500
+++ ccache-3.1.7.patched/ccache.c       2012-07-20 13:41:21.000000000 -0400
@@ -387,13 +387,31 @@
 make_relative_path(char *path)
 {
        char *relpath;
+       char *canon_path;
+       char *canon_cwd;
+       bool cwd_canonicalized = false;
 
        if (!base_dir || !str_startswith(path, base_dir)) {
                return path;
        }
 
-       relpath = get_relative_path(current_working_dir, path);
+       /* Canonicalize the current working directory if it has not been
+        * canonicalized already.
+        */
+       if (!cwd_canonicalized) {
+               canon_cwd = x_realpath(current_working_dir);
+               free(current_working_dir);
+
+               current_working_dir = canon_cwd;
+
+               cwd_canonicalized = true;
+       }
+
+       canon_path = x_realpath(path);
+
+       relpath = get_relative_path(current_working_dir, canon_path);
        free(path);
+       free(canon_path);
        return relpath;
 }
 
_______________________________________________
ccache mailing list
ccache@lists.samba.org
https://lists.samba.org/mailman/listinfo/ccache

Reply via email to