Re: [PATCH 2-3/3] Name make_*_path functions more accurately

["Carlos Martín Nieto" <cmn@elego.de>]

On Wed, 2011-03-16 at 17:59 +0100, Michael J Gruber wrote:
> Carlos Martín Nieto venit, vidit, dixit 16.03.2011 17:51:
>>[...]
> > -const char *make_nonrelative_path(const char *path)
> > +/*
> > + * Use this to get an absolute path from a relative one. If you want
> > + * to resolve links, you should use real_path.
> > + *
> > + * If the path is already absolute, then return path. As the user is
> > + * never meant to free the return value, we're safe.
> > + */
> > +const char *absolute_path(const char *path)
> >  {
> >  	static char buf[PATH_MAX + 1];
> >  
> >  	if (is_absolute_path(path)) {
> > -		if (strlcpy(buf, path, PATH_MAX) >= PATH_MAX)
> > +		if (strlen(path) >= PATH_MAX)
> >  			die("Too long path: %.*s", 60, path);
> > +		else
> > +			return path;
> >  	} else {
> >  		size_t len;
> >  		const char *fmt;
> 
> This is not simply a renaming change that you're sneaking in here. What
> is it about?

 Gah, I've been staring at this patch too long. This is related another
thread where Junio said it'd be a good idea to change the semantics so
we centralise checking wether we already have an absolute path. See
http://thread.gmane.org/gmane.comp.version-control.git/169012/focus=169063 (Message-ID: <7vpqptb976.fsf@alter.siamese.dyndns.org>).

>>[...]
> > diff --git a/dir.c b/dir.c
> > index 570b651..5a9372a 100644
> > --- a/dir.c
> > +++ b/dir.c
> > @@ -1023,8 +1023,8 @@ char *get_relative_cwd(char *buffer, int size, const char *dir)
> >  	if (!getcwd(buffer, size))
> >  		die_errno("can't find the current directory");
> >  
> > -	if (!is_absolute_path(dir))
> > -		dir = make_absolute_path(dir);
> > +	/* getcwd resolves links and gives us the real path */
> > +	dir = real_path(dir);
> 
> Why remove the check? Again, this is not just renaming.

 Semantics change. Same as above. The one below (snipped) is the same.

>>[...]
> I think you should strictly separate the renaming issue from other
> patches (and describe/motivate the latter). It's fine to have a large
> patch with mechanical changes (renaming) if you stick to just those.

 Unfortunately I introduced some modifications trying to reduce the
number of times we call make_absolute_path/real_path and they snuck in.
Sorry.

   cmn