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

["Torsten Bögershausen" <tboegi@web.de>]

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@gmail.com>
> ---
>  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