Re: [PATCHv6 1/4] Read (but not write) from $XDG_CONFIG_HOME/git/config file

+char *mkpathdup(const char *fmt, ...)
+{
+	char path[PATH_MAX];
+	va_list args;
+	unsigned len;
+
+	va_start(args, fmt);
+	len = vsnprintf(path, sizeof(path), fmt, args);
+	va_end(args);
+	if (len >= sizeof(path))
+		return xstrdup(bad_path);
+	return xstrdup(cleanup_path(path));
+}

Hrmph. If a new helper is introduced anyway, wouldn't it be a better
idea to get rid of the hardcoded PATH_MAX limitation, perhaps using
strbuf_vaddf() or something in the implementation of this function?

"rm -f .gitconfig"; always consider the possibility that any part of
the previous test could have been omitted or failed.

Junio C Hamano [off-list ref] a écrit :

quoted

+char *mkpathdup(const char *fmt, ...)
+{
+	char path[PATH_MAX];
+	va_list args;
+	unsigned len;
+
+	va_start(args, fmt);
+	len = vsnprintf(path, sizeof(path), fmt, args);
+	va_end(args);
+	if (len >= sizeof(path))
+		return xstrdup(bad_path);
+	return xstrdup(cleanup_path(path));
+}

Hrmph. If a new helper is introduced anyway, wouldn't it be a better
idea to get rid of the hardcoded PATH_MAX limitation, perhaps using
strbuf_vaddf() or something in the implementation of this function?

Ramsay Jones, what do you think about this ?

Junio C Hamano [off-list ref] a écrit :

quoted

+char *mkpathdup(const char *fmt, ...)
+{
+	char path[PATH_MAX];
+	va_list args;
+	unsigned len;
+
+	va_start(args, fmt);
+	len = vsnprintf(path, sizeof(path), fmt, args);
+	va_end(args);
+	if (len >= sizeof(path))
+		return xstrdup(bad_path);
+	return xstrdup(cleanup_path(path));
+}

Hrmph. If a new helper is introduced anyway, wouldn't it be a better
idea to get rid of the hardcoded PATH_MAX limitation, perhaps using
strbuf_vaddf() or something in the implementation of this function?

What about this ?

char *mkpathdup(const char *fmt, ...)
{
	char *path;
	struct strbuf sb = STRBUF_INIT;
	va_list args;

	va_start(args, fmt);
	strbuf_vaddf(&sb, fmt, args);
	va_end(args);
	path = sb.buf;

	strbuf_release(&sb);
	return xstrdup(cleanup_path(path));
}

On Fri, Jun 8, 2012 at 2:26 PM,  [off-list ref] wrote:

Junio C Hamano [off-list ref] a écrit :

quoted

quoted

+char *mkpathdup(const char *fmt, ...)
+{
+       char path[PATH_MAX];
+       va_list args;
+       unsigned len;
+
+       va_start(args, fmt);
+       len = vsnprintf(path, sizeof(path), fmt, args);
+       va_end(args);
+       if (len >= sizeof(path))
+               return xstrdup(bad_path);
+       return xstrdup(cleanup_path(path));
+}

Hrmph. If a new helper is introduced anyway, wouldn't it be a better
idea to get rid of the hardcoded PATH_MAX limitation, perhaps using
strbuf_vaddf() or something in the implementation of this function?

What about this ?


char *mkpathdup(const char *fmt, ...)
{
       char *path;
       struct strbuf sb = STRBUF_INIT;
       va_list args;

       va_start(args, fmt);
       strbuf_vaddf(&sb, fmt, args);
       va_end(args);
       path = sb.buf;

       strbuf_release(&sb);
       return xstrdup(cleanup_path(path));

}

No, strbuf_release(&sb) frees 'sb.buf', causing 'path' to point to
unallocated memory. You can fix that by doing something along these
lines on top:

 	va_end(args);
-	path = sb.buf;
+	path = xstrdup(cleanup_path(path));

 	strbuf_release(&sb);
-	return xstrdup(cleanup_path(path));
+	return path;
 }

Erik Faye-Lund [off-list ref] a écrit :

quoted

char *mkpathdup(const char *fmt, ...)
{
       char *path;
       struct strbuf sb = STRBUF_INIT;
       va_list args;

       va_start(args, fmt);
       strbuf_vaddf(&sb, fmt, args);
       va_end(args);
       path = sb.buf;

       strbuf_release(&sb);
       return xstrdup(cleanup_path(path));

}

No, strbuf_release(&sb) frees 'sb.buf', causing 'path' to point to
unallocated memory. You can fix that by doing something along these
lines on top:

 	va_end(args);
-	path = sb.buf;
+	path = xstrdup(cleanup_path(path));

 	strbuf_release(&sb);
-	return xstrdup(cleanup_path(path));
+	return path;
 }

You are right, but I think you rather mean this, no?

  	va_end(args);
-	path = sb.buf;
+	path = xstrdup(cleanup_path(sb.buf));

  	strbuf_release(&sb);
-	return xstrdup(cleanup_path(path));
+	return path;
  }

On Fri, Jun 8, 2012 at 2:54 PM,  [off-list ref] wrote:

Erik Faye-Lund [off-list ref] a écrit :

quoted

quoted

char *mkpathdup(const char *fmt, ...)
{
       char *path;
       struct strbuf sb = STRBUF_INIT;
       va_list args;

       va_start(args, fmt);
       strbuf_vaddf(&sb, fmt, args);
       va_end(args);
       path = sb.buf;

       strbuf_release(&sb);
       return xstrdup(cleanup_path(path));

}

No, strbuf_release(&sb) frees 'sb.buf', causing 'path' to point to
unallocated memory. You can fix that by doing something along these
lines on top:

       va_end(args);
-       path = sb.buf;
+       path = xstrdup(cleanup_path(path));

       strbuf_release(&sb);
-       return xstrdup(cleanup_path(path));
+       return path;
 }

You are right, but I think you rather mean this, no?


       va_end(args);
-       path = sb.buf;
+       path = xstrdup(cleanup_path(sb.buf));


       strbuf_release(&sb);
-       return xstrdup(cleanup_path(path));
+       return path;
 }

Yes, sorry for the fat fingers :)

nguyenhu@minatec.inpg.fr wrote:

Junio C Hamano [off-list ref] a écrit :

quoted

quoted

+char *mkpathdup(const char *fmt, ...)
+{
+	char path[PATH_MAX];
+	va_list args;
+	unsigned len;
+
+	va_start(args, fmt);
+	len = vsnprintf(path, sizeof(path), fmt, args);
+	va_end(args);
+	if (len >= sizeof(path))
+		return xstrdup(bad_path);
+	return xstrdup(cleanup_path(path));
+}

Hrmph. If a new helper is introduced anyway, wouldn't it be a better
idea to get rid of the hardcoded PATH_MAX limitation, perhaps using
strbuf_vaddf() or something in the implementation of this function?

Ramsay Jones, what do you think about this ?

I think that I'm sorry for the late reply, I've been away ...  :-D

I noticed your new series (v7 I think) which looked good (as far as the
mkpathdup() implementation is concerned) and I don't think it will tickle
the cygwin bug. (I haven't actually fetched that mail yet, I've only read
it using my ISPs web-mail interface, but will do so soon and test it on
cygwin).

Thanks!

ATB,
Ramsay Jones