Re: [PATCH 2/4] Add functions get_relative_cwd() and is_inside_dir()

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi,

On Tue, 31 Jul 2007, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >
> >> The function get_relative_cwd() works just as getcwd(), only that it
> >> takes an absolute path as additional parameter, returning the prefix
> >> of the current working directory relative to the given path.  If the
> >> cwd is no subdirectory of the given path, it returns NULL.
> >> ...
> >> +/*
> >> + * get_relative_cwd() gets the prefix of the current working directory
> >> + * relative to 'dir'.  If we are not inside 'dir', it returns NULL.
> >> + * As a convenience, it also returns NULL if 'dir' is already NULL.
> >> + */
> >> +char *get_relative_cwd(char *buffer, int size, const char *dir)
> >> +{
> >> +	char *cwd = buffer;
> >> +
> >> +	if (!dir || !getcwd(buffer, size))
> >> +		return NULL;
> >
> > When is it not a fatal error if get_relative_cwd() is called
> > with a NULL dir parameter, or getcwd() fails?
> >
> > If there is no valid such cases, I would rather have this
> > die(), former with "BUG" and the latter with strerror(errno).
> 
> Heh, it turns out that there is this lazy or clever (depending
> on the viewpoint) caller that passes the return value of
> get_git_work_tree() to this function and expect this to return
> NULL when no work tree is found.

Right.  I thought I had said that (something along the lines: it is more 
convenient not having to check the directory), but I probably did not.

> The callers of the is_* functions are much cleaner and in that sense the 
> series is a definite improvement, but this one particular obscurity 
> makes me wonder if it is replacing one unholy mess with a smaller but 
> still unholy mess.
> 
> Will apply on "master" and will be part of -rc4, but we probably would 
> want to have a longer pre-final freeze than usual to really make sure 
> this one is good.

I'll provide a patch which makes the callers of get_relative_cwd() holy, 
and skip the check in get_relative_cwd(), okay?

Ciao,
Dscho