Hi, It seems you forgot to make cwd_canonicalized static? Currently, it has no effect.
Also, the test suite crashes with your patch. I recommend running the test suite during development even for minor patches (and for bonus points: add new tests for the new functionality :-)). The crash happens because realpath returns NULL for nonexisting paths, and the test suite sets up fake paths for some parameters; this could happen in real usage as well. So, the code needs to handle x_realpath returning NULL. Thinking more about this, it would probably be better to (instead of realpath) use some function that doesn't expand symlinks and doesn't care whether the path exists or not. I'm not aware of such an existing (and portable) function, though, so we likely will have to write it ourselves. -- Joel On 20 July 2012 19:45, Eric Blau <eric.b...@tekelec.com> wrote: > 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 > > > _______________________________________________ > ccache mailing list > ccache@lists.samba.org > https://lists.samba.org/mailman/listinfo/ccache > _______________________________________________ ccache mailing list ccache@lists.samba.org https://lists.samba.org/mailman/listinfo/ccache