Re: [PATCH 2/2] setup: Don't dereference in-tree symlinks for absolute paths

[Martin Erik Werner <martinerikwerner@gmail.com>]

On Sun, Jan 26, 2014 at 06:19:25PM +0100, Torsten Bögershausen wrote:
> On 2014-01-26 15.22, Martin Erik Werner wrote:
> > The prefix_path_gently() function currently applies real_path to
> > everything if given an absolute path, dereferencing symlinks both
> > outside and inside the work tree. In order to manipulate symliks in the
> > work tree using absolute paths, symlinks should only be dereferenced
> > outside the work tree.
> > 
> > Modify prefix_path_gently() to first normalize the path in order to
> > make sure path levels are separated by '/', then use this separator to
> > check the real path of each level of the path until it has found the
> > length that corresponds to the work tree.
> > 
> > For absolute paths, the function did not, nor does now do, any actual
> > prefixing, hence we simply remove the path corresponding to the work
> > tree and return the remaining in-tree part of the path.
> > 
> > Fixes t0060-82.
> > 
> > Signed-off-by: Martin Erik Werner <martinerikwerner@gmail.com>
> > ---
> >  setup.c               | 54 ++++++++++++++++++++++++++++++++-------------------
> >  t/t0060-path-utils.sh |  2 +-
> >  2 files changed, 35 insertions(+), 21 deletions(-)
> > 
> > diff --git a/setup.c b/setup.c
> > index 6c3f85f..bec587e 100644
> > --- a/setup.c
> > +++ b/setup.c
> > @@ -22,11 +22,41 @@ char *prefix_path_gently(const char *prefix, int len,
> >  	const char *orig = path;
> >  	char *sanitized;
> >  	if (is_absolute_path(orig)) {
> > -		const char *temp = real_path(path);
> > -		sanitized = xmalloc(len + strlen(temp) + 1);
> > -		strcpy(sanitized, temp);
> > +		char npath[strlen(path)];
> Is this portable ?
> This is variable-length array, isn't it ?
> Using xmalloc() may be better
Ah, right, that looks bad now that you mention it.
> >  		if (remaining_prefix)
> >  			*remaining_prefix = 0;
> > +		if (normalize_path_copy_len(npath, path, remaining_prefix))
> > +			return NULL;
> > +		const char *work_tree = get_git_work_tree();
> declaration after statements should be avoided (not only here)
> 
Indeed, I somehow guessed that declaration-after-statement was ok and
tried to keep them close to usage, bad guess, evidently.

I've rerolled the last v1 patch accordingly:
* Use xmalloc() when initializing char* from strlen()
* Separate and move declarations to beginning of scope
* Fix a strcpy that should've been a strcat (which would've nuked DOS
  prefixes, I think)

  [PATCH v2 2/2] setup: Don't dereference in-tree symlinks for absolute paths

 setup.c               | 64 +++++++++++++++++++++++++++++++++++----------------
 t/t0060-path-utils.sh |  2 +-
 2 files changed, 45 insertions(+), 21 deletions(-)

--
Martin Erik Werner <martinerikwerner@gmail.com>