[PATCH] notes_merge_commit(): do not pass temporary buffer to other function

[PATCH] notes_merge_commit(): do not pass temporary buffer to other function

[Michael Haggerty]

It is unsafe to pass a temporary buffer as an argument to
read_directory().

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---

I discovered this problem when an innocent modification to unrelated
code triggered test failures.

 notes-merge.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git notes-merge.c notes-merge.c
index e1aaf43..baaf31f 100644
--- notes-merge.c
+++ notes-merge.c
@@ -680,7 +680,7 @@ int notes_merge_commit(struct notes_merge_options *o,
 	 * Finally store the new commit object SHA1 into 'result_sha1'.
 	 */
 	struct dir_struct dir;
-	const char *path = git_path(NOTES_MERGE_WORKTREE "/");
+	char *path = xstrdup(git_path(NOTES_MERGE_WORKTREE "/"));
 	int path_len = strlen(path), i;
 	const char *msg = strstr(partial_commit->buffer, "\n\n");
 
@@ -720,6 +720,7 @@ int notes_merge_commit(struct notes_merge_options *o,
 			    result_sha1);
 	OUTPUT(o, 4, "Finalized notes merge commit: %s",
 	       sha1_to_hex(result_sha1));
+	free(path);
 	return 0;
 }
 
-- 
1.7.7.rc2

Re: [PATCH] notes_merge_commit(): do not pass temporary buffer to other function

[Johan Herland]

On Tue, Sep 27, 2011 at 06:46, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> It is unsafe to pass a temporary buffer as an argument to
> read_directory().
>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>

ACK.

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

Re: [PATCH] notes_merge_commit(): do not pass temporary buffer to other function

[Junio C Hamano]

Michael Haggerty <mhagger@alum.mit.edu> writes:

> I discovered this problem when an innocent modification to unrelated
> code triggered test failures.
>
>  notes-merge.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git notes-merge.c notes-merge.c
> index e1aaf43..baaf31f 100644
> --- notes-merge.c
> +++ notes-merge.c

It is Ok to play with -p0 yourself but please don't do that in the public.

> @@ -680,7 +680,7 @@ int notes_merge_commit(struct notes_merge_options *o,
>  	 * Finally store the new commit object SHA1 into 'result_sha1'.
>  	 */
>  	struct dir_struct dir;
> -	const char *path = git_path(NOTES_MERGE_WORKTREE "/");
> +	char *path = xstrdup(git_path(NOTES_MERGE_WORKTREE "/"));
>  	int path_len = strlen(path), i;
>  	const char *msg = strstr(partial_commit->buffer, "\n\n");
>  
> @@ -720,6 +720,7 @@ int notes_merge_commit(struct notes_merge_options *o,
>  			    result_sha1);
>  	OUTPUT(o, 4, "Finalized notes merge commit: %s",
>  	       sha1_to_hex(result_sha1));
> +	free(path);
>  	return 0;
>  }

Re: [PATCH] notes_merge_commit(): do not pass temporary buffer to other function

[Michael Haggerty]

On 09/27/2011 06:59 PM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>> I discovered this problem when an innocent modification to unrelated
>> code triggered test failures.
>>
>>  notes-merge.c |    3 ++-
>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git notes-merge.c notes-merge.c
>> index e1aaf43..baaf31f 100644
>> --- notes-merge.c
>> +++ notes-merge.c
> 
> It is Ok to play with -p0 yourself but please don't do that in the public.

Sorry; I had set diff.noprefix=true, not realizing that it would affect
things like "git format-patch".  It also confused emacs' magit mode, and
probably some other tools.  It's now set permanently back to false.

The reason I was experimenting with this option is that it is a quick
double-click to select a filename like "foo/bar" in the diff output,
whereas selecting the filename out of "a/foo/bar" requires a slower
click and drag.  Once I considered whether git could be taught to
explicitly ignore the "[ab]/" prefix when parsing filenames, but that is
too evil even for me :-)

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: [PATCH] notes_merge_commit(): do not pass temporary buffer to other function

[Junio C Hamano]

Michael Haggerty <mhagger@alum.mit.edu> writes:

>>> diff --git notes-merge.c notes-merge.c
>>> index e1aaf43..baaf31f 100644
>>> --- notes-merge.c
>>> +++ notes-merge.c
>> 
>> It is Ok to play with -p0 yourself but please don't do that in the public.
>
> Sorry; I had set diff.noprefix=true, not realizing that it would affect
> things like "git format-patch".  It also confused emacs' magit mode, and
> probably some other tools.  It's now set permanently back to false.

Thanks.

It is nothing catastrophic (I applied it with "am -p0 -s3c" just fine),
but everybody here sends -p1 format, so...