Re: [PATCH 1/3] make_absolute_path: Don't try to copy a string to itself
On lun, 2011-03-14 at 15:58 -0700, Junio C Hamano wrote:
Show 15 quoted lines
> Carlos Martín Nieto <cmn@elego.de> 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)
Show 6 quoted lines
>
> > 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
Carlos Martín Nieto <cmn@elego.de>