Re: [PATCH v2 01/21] path.c: avoid PATH_MAX as buffer size from get_pathname()

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

 



On 2013-12-14 11.54, Nguyễn Thái Ngọc Duy wrote:
> We've been avoiding PATH_MAX whenever possible. This patch avoids
> PATH_MAX in get_pathname() and gives it enough room not to worry about
> really long paths.
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
>  path.c | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/path.c b/path.c
> index 24594c4..4c1c144 100644
> --- a/path.c
> +++ b/path.c
> @@ -16,10 +16,11 @@ static int get_st_mode_bits(const char *path, int *mode)
>  
>  static char bad_path[] = "/bad-path/";
>  
> -static char *get_pathname(void)
> +static char *get_pathname(size_t *len)
>  {
> -	static char pathname_array[4][PATH_MAX];
> +	static char pathname_array[4][4096];
The PATH_MAX doesn't seem to be that bad:
http://pubs.opengroup.org/onlinepubs/009695399/basedefs/limits.h.html
Or do we have a an OS where PATH_MAX does not work ?

Windows uses MAX_PATH=260 PATH_MAX=259
<http://msdn.microsoft.com/en-us/library/windows/desktop/aa365247%28v=vs.85%29.aspx>

Which means that the current implementation of git can not use path names longer than
259 (260 including the trailing \0)
(Please correct me if this is wrong)

Which means that the code working with the buffers must make sure to stay within range,
and not to write beyond the buffers.

If we really want to go away from PATH_MAX, is a hard-coded value of 4096 so attractive ?
Because we can either

a) Re-define PATH_MAX in git-compat-util.h
b) Use an own  #define in git-compat-util.h, like e.g. GIT_PATH_MAX
c) Change the code to use a "strbuf" which can grow on demand.

I would prefer c) over b) over a)




>  	static int index;
> +	*len = sizeof(pathname_array[0]);
>  	return pathname_array[3 & ++index];
>  }
>  
> @@ -108,24 +109,26 @@ char *mkpath(const char *fmt, ...)
>  {
>  	va_list args;
>  	unsigned len;
> -	char *pathname = get_pathname();
> +	size_t n;
> +	char *pathname = get_pathname(&n);
>  
>  	va_start(args, fmt);
> -	len = vsnprintf(pathname, PATH_MAX, fmt, args);
> +	len = vsnprintf(pathname, n, fmt, args);
Renaming "n" into something like "max" or "max_len" could
make this line 5% easier to read.
(And similar at some places below)
/Torsten
--
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]