[PATCH] fix use of sometimes uninitialized variable 'msg'

[PATCH] fix use of sometimes uninitialized variable 'msg'

[Michael Coleman]

[PATCH] fix uninitialized use of msg buffer

[Michael Coleman]

---
 builtin-branch.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-branch.c b/builtin-branch.c
index d60690b..ac64b99 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -315,7 +315,7 @@ static void create_branch(const char *name, const char *start_name,
 	struct ref_lock *lock;
 	struct commit *commit;
 	unsigned char sha1[20];
-	char ref[PATH_MAX], msg[PATH_MAX + 20];
+	char ref[PATH_MAX], msg[PATH_MAX + 20] = { 0 };
 
 	snprintf(ref, sizeof ref, "refs/heads/%s", name);
 	if (check_ref_format(ref))
-- 
1.5.0.rc3

[PATCH] builtin-branch: be prepared for ref-logging

[Lars Hjemli]

The test for reflog parameter to create_branch() should also have tested
for log_all_ref_updates. But there is no good reason not to prepare a
reflog message, so lets just do it.

Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---

If log_all_ref_updates==1 then the reflog will get an empty message with 
your patch. That is better than using an uninitialized message, but I think
it's still better to just prepare a proper message.


 builtin-branch.c |    7 +------
 1 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/builtin-branch.c b/builtin-branch.c
index d60690b..16b589e 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -342,12 +342,7 @@ static void create_branch(const char *name, const char *start_name,
 	if (!lock)
 		die("Failed to lock ref for update: %s.", strerror(errno));
 
-	if (reflog) {
-		log_all_ref_updates = 1;
-		snprintf(msg, sizeof msg, "branch: Created from %s",
-			 start_name);
-	}
-
+	snprintf(msg, sizeof msg, "branch: Created from %s", start_name);
 	if (write_ref_sha1(lock, sha1, msg) < 0)
 		die("Failed to write ref: %s.", strerror(errno));
 }
-- 
1.5.0.rc2.ge4b0e

Re: [PATCH] builtin-branch: be prepared for ref-logging

[Junio C Hamano]

Lars Hjemli <hjemli@gmail.com> writes:

> The test for reflog parameter to create_branch() should also have tested
> for log_all_ref_updates. But there is no good reason not to prepare a
> reflog message, so lets just do it.
>
> Signed-off-by: Lars Hjemli <hjemli@gmail.com>
> ---
>
> If log_all_ref_updates==1 then the reflog will get an empty message with 
> your patch. That is better than using an uninitialized message, but I think
> it's still better to just prepare a proper message.

True, but don't you still need to set log_all_ref_updates while
you call write_ref_sha1() for that ref in order to make sure
that log_ref_write() sets O_CREAT in oflags?

[PATCH] builtin-branch: be prepared for ref-logging

[Lars Hjemli]

The test for reflog parameter to create_branch() should also have tested
for log_all_ref_updates. But there is no good reason not to prepare a
reflog message, so lets just do it.

Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---

Sorry, the previous patch was obviously no good, since it didn't honour
the case where (reflog==1 && log_all_ref_updates==0)

This one should be better.


 builtin-branch.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/builtin-branch.c b/builtin-branch.c
index d60690b..a8c8c98 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -342,12 +342,10 @@ static void create_branch(const char *name, const char *start_name,
 	if (!lock)
 		die("Failed to lock ref for update: %s.", strerror(errno));
 
-	if (reflog) {
+	if (reflog)
 		log_all_ref_updates = 1;
-		snprintf(msg, sizeof msg, "branch: Created from %s",
-			 start_name);
-	}
 
+	snprintf(msg, sizeof msg, "branch: Created from %s", start_name);
 	if (write_ref_sha1(lock, sha1, msg) < 0)
 		die("Failed to write ref: %s.", strerror(errno));
 }
-- 
1.5.0.rc2.ge4b0e

Re: [PATCH] builtin-branch: be prepared for ref-logging

[Junio C Hamano]

Lars Hjemli <hjemli@gmail.com> writes:

> The test for reflog parameter to create_branch() should also have tested
> for log_all_ref_updates. But there is no good reason not to prepare a
> reflog message, so lets just do it.
>
> Signed-off-by: Lars Hjemli <hjemli@gmail.com>
> ---
>
> Sorry, the previous patch was obviously no good, since it didn't honour
> the case where (reflog==1 && log_all_ref_updates==0)

Ah, I did not see your fix-up patch before I sent out my
comments.

Funny thing is that I thought I fixed this already, but
apparently I didn't.  It could have been one of my "fixing in
working tree, sending out a patch to the list for comments and
then forgetting to apply it".  Maybe not.

In any case, thanks for the fix.

Re: [PATCH] builtin-branch: be prepared for ref-logging

[Lars Hjemli]

On 2/3/07, Junio C Hamano <junkio@cox.net> wrote:
> Funny thing is that I thought I fixed this already

Infact, you did:

  http://article.gmane.org/gmane.comp.version-control.git/37197

Don't know what happend to this one...

-- 
larsh

Re: [PATCH] builtin-branch: be prepared for ref-logging

[Junio C Hamano]

"Lars Hjemli" <hjemli@gmail.com> writes:

> On 2/3/07, Junio C Hamano <junkio@cox.net> wrote:
>> Funny thing is that I thought I fixed this already
>
> Infact, you did:
>
>  http://article.gmane.org/gmane.comp.version-control.git/37197
>
> Don't know what happend to this one...

Thanks, and I do think that patch is better ;-).

Re: [PATCH] builtin-branch: be prepared for ref-logging

[Lars Hjemli]

On 2/3/07, Junio C Hamano <junkio@cox.net> wrote:
> "Lars Hjemli" <hjemli@gmail.com> writes:
>
> > On 2/3/07, Junio C Hamano <junkio@cox.net> wrote:
> >> Funny thing is that I thought I fixed this already
> >
> > Infact, you did:
> >
> >  http://article.gmane.org/gmane.comp.version-control.git/37197
> >
> > Don't know what happend to this one...
>
> Thanks, and I do think that patch is better ;-).

Agreed

-- 
larsh

Re: [PATCH] builtin-branch: be prepared for ref-logging

[Junio C Hamano]

"Lars Hjemli" <hjemli@gmail.com> writes:

> On 2/3/07, Junio C Hamano <junkio@cox.net> wrote:
>> Funny thing is that I thought I fixed this already
>
> Infact, you did:
>
>  http://article.gmane.org/gmane.comp.version-control.git/37197
>
> Don't know what happend to this one...

It turns out that I was missing another patch from the same day.

	http://article.gmane.org/gmane.comp.version-control.git/37238

I guess we need a patch secretary whose job is to keep track of
all the patches sent to the list and make sure nothing stays in
limbo X-<.

Re: [PATCH] fix use of sometimes uninitialized variable 'msg'

[Mike Coleman]

Michael Coleman <tutufan <at> gmail.com> writes:
> 
> 
> 

Oops--I didn't realize a header email was going to be sent out, too.  Not quite
sure what happened there.  When I tested it before, I thought only one email got
sent.

Mike