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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> Johannes Schindelin <Johannes.Schindelin@xxxxxx> 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.

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.

-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux