threads / patch / 6641
patchfix use of sometimes uninitialized variable 'msg'
Subject: [PATCH] fix use of sometimes uninitialized variable 'msg'
[PATCH] fix uninitialized use of msg buffer
--- builtin-branch.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
Show changes to builtin-branch.c +1 −1
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
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(-)
Show changes to builtin-branch.c +1 −6
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
Junio C HamanoRe: [PATCH] builtin-branch: be prepared for ref-logging
Lars Hjemli <hjemli@gmail.com> writes:
Show 10 quoted lines
> 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
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(-)
Show changes to builtin-branch.c +2 −4
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
Junio C HamanoRe: [PATCH] builtin-branch: be prepared for ref-logging
Lars Hjemli <hjemli@gmail.com> writes:
Show 9 quoted lines
> 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
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
Junio C HamanoRe: [PATCH] builtin-branch: be prepared for ref-logging
"Lars Hjemli" <hjemli@gmail.com> writes:
Show 8 quoted lines
> 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
On 2/3/07, Junio C Hamano <junkio@cox.net> wrote:
Show 12 quoted lines
> "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
Junio C HamanoRe: [PATCH] builtin-branch: be prepared for ref-logging
"Lars Hjemli" <hjemli@gmail.com> writes:
Show 8 quoted lines
> 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'
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