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

nguyenhu@minatec.inpg.fr writes:

quoted

Modulo

	path = strbuf_detach(&sb, NULL);

that is more or less what I meant.

So now the mkpathdup() function looks like:

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 = strbuf_detach(&sb, NULL);

	strbuf_release(&sb);
	return path;
}

This new variation of mkpathdup() function both fix the bug addressed
by commit 05bab3ea and avoid the use of bounded buffer.

I didn't mean to suggest removing the call to clean-up-path
function.  What I meant was that strbuf_detach() is a way to take
the ownership of the buffer, so that you do not have to call
strbuf_release() on it.

Junio C Hamano [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 = strbuf_detach(&sb, NULL);

	strbuf_release(&sb);
	return path;
}

I didn't mean to suggest removing the call to clean-up-path
function.  What I meant was that strbuf_detach() is a way to take
the ownership of the buffer, so that you do not have to call
strbuf_release() on it.

So with the call to clean-up-path function and without the call to
strbuf_release(), mkpathdup() function becomes :

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

	va_start(args, fmt);
	strbuf_vaddf(&sb, fmt, args);
	va_end(args);

	return cleanup_path(strbuf_detach(&sb, NULL));
}

On Sun, Jun 10, 2012 at 3:48 PM,  [off-list ref] wrote:

Junio C Hamano [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 = strbuf_detach(&sb, NULL);

       strbuf_release(&sb);
       return path;
}

I didn't mean to suggest removing the call to clean-up-path
function.  What I meant was that strbuf_detach() is a way to take
the ownership of the buffer, so that you do not have to call
strbuf_release() on it.

So with the call to clean-up-path function and without the call to
strbuf_release(), mkpathdup() function becomes :


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

       va_start(args, fmt);
       strbuf_vaddf(&sb, fmt, args);
       va_end(args);

       return cleanup_path(strbuf_detach(&sb, NULL));

}

The awkward thing about doing this, is that the memory allocated by
the strbuf cannot be reclaimed if you go with this. A pointer that has
been adjusted (like cleanup_path can do) cannot be successfully fed to
free.

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

quoted

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

       va_start(args, fmt);
       strbuf_vaddf(&sb, fmt, args);
       va_end(args);

       return cleanup_path(strbuf_detach(&sb, NULL));

}

The awkward thing about doing this, is that the memory allocated by
the strbuf cannot be reclaimed if you go with this. A pointer that has
been adjusted (like cleanup_path can do) cannot be successfully fed to
free.

Do you mean that the previous version is preferable in keeping  
clean-up-path function ?

On Sun, Jun 10, 2012 at 10:02 PM,  [off-list ref] wrote:

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

quoted

quoted

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

       va_start(args, fmt);
       strbuf_vaddf(&sb, fmt, args);
       va_end(args);

       return cleanup_path(strbuf_detach(&sb, NULL));

}

The awkward thing about doing this, is that the memory allocated by
the strbuf cannot be reclaimed if you go with this. A pointer that has
been adjusted (like cleanup_path can do) cannot be successfully fed to
free.

Do you mean that the previous version is preferable in keeping clean-up-path
function ?

No, that wasn't my intention. Since this is only used for a few config
files, I don't think leaking the memory is a big deal. But it's
probably worth putting a comment in the code about it, to warn
potential future users.