On lun, 2011-03-14 at 15:58 -0700, Junio C Hamano wrote: > Carlos MartÃn Nieto <cmn@xxxxxxxx> writes: > > >> I don't think it is a bad idea per-se to avoid a copy from the same memory > >> location into the same memory location, but independent of the necessity > >> of fixes at the low-level, shouldn't we fix the callers that do not check > >> if what they have is already absolute? > > > > If we'd like the semantics to be "whatever I had, I now know what the > > absolute path is" then we could make the check in the beginning of the > > function, to centralise the check. If the semantics should be "I don't > > have an absolute path, so I need to figure out what it is", then there > > should be a check before calling make_absolute_path() (the name suggests > > the second). > > Good thinking, and I think the former semantics would be easier to use. I was asking about why we didn't just use realpath, but it seems there is no portable way of dealing with it (even without counting NFS over different systems). I'm thinking of renaming make_absolute_path to real_path as it's in fact our implementation of realpath (and actually describes what it does), without a is_absolute_path check, as a path could be absolute but point to a link, adding a comment to say that if you don't mind links, you should use make_nonrelative_path (I'd rename it to absolute_path, but that may be too close to the old make_absolute_path) > > > There is however the extra functionality the function offers, namely > > resolving links. It might be good to split it into two functions so each > > caller can specify what it wants. > > Probably. With the changes mentioned earlier, if you want an absolute pathname, you'd call absolute_path/make_nonrelative_path and if you want to make sure you have the real path of the target file, you'd use real_path just as you'd use realpath on a sane system, with cmn -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html