Re: [PATCH 1/3] make_absolute_path: Don't try to copy a string to itself

3 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 1/3] make_absolute_path: Don't try to copy a string to itself

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:50:46

Carlos Martín Nieto [off-list ref] writes:
quoted
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.
 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.

Re: [PATCH 1/3] make_absolute_path: Don't try to copy a string to itself

From: Carlos Martín Nieto <hidden>
Date: 2016-06-15 22:50:46

On lun, 2011-03-14 at 15:58 -0700, Junio C Hamano wrote:
Carlos Martín Nieto [off-list ref] writes:
quoted
quoted
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)
quoted
 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

Re: [PATCH 1/3] make_absolute_path: Don't try to copy a string to itself

From: Carlos Martín Nieto <hidden>
Date: 2016-06-15 22:50:46

On mar, 2011-03-15 at 12:59 +0100, Carlos Martín Nieto wrote:
On lun, 2011-03-14 at 15:58 -0700, Junio C Hamano wrote:
quoted
Carlos Martín Nieto [off-list ref] writes:
[...]
quoted
quoted
 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
 ... a comment on the functions and maybe some documentation in
Documentation/techncal, as it doesn't seem to exist yet.

   cmn
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help