* BUG: Git command causes crash @ 2011-12-18 3:13 Ryan O'Hara 2011-12-18 5:03 ` Jeff King 0 siblings, 1 reply; 4+ messages in thread From: Ryan O'Hara @ 2011-12-18 3:13 UTC (permalink / raw) To: git On Git for Windows (MinGW), at least, this command causes git to crash: git commit -a --no-message --dry-run ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: BUG: Git command causes crash 2011-12-18 3:13 BUG: Git command causes crash Ryan O'Hara @ 2011-12-18 5:03 ` Jeff King 2011-12-18 5:07 ` Jeff King 0 siblings, 1 reply; 4+ messages in thread From: Jeff King @ 2011-12-18 5:03 UTC (permalink / raw) To: Ryan O'Hara; +Cc: git, Junio C Hamano On Sat, Dec 17, 2011 at 07:13:53PM -0800, Ryan O'Hara wrote: > On Git for Windows (MinGW), at least, this command causes git to crash: > > git commit -a --no-message --dry-run On Linux, too, using just "git commit --no-message" (whether there is something to commit or not). This fixes it for me. -- >8 -- Subject: [PATCH] commit: initialize static strbuf Strbufs cannot rely on static all-zero initialization; instead, they must use STRBUF_INIT to point to the "slopbuf". Without this patch, "git commit --no-message" segfaults reliably. Signed-off-by: Jeff King <peff@peff.net> --- builtin/commit.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index d0f27f9..336faff 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -104,7 +104,7 @@ static int use_editor = 1, include_status = 1; static int show_ignored_in_status; static const char *only_include_assumed; -static struct strbuf message; +static struct strbuf message = STRBUF_INIT; static int null_termination; static enum { -- 1.7.8.rc3.14.gd2470 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: BUG: Git command causes crash 2011-12-18 5:03 ` Jeff King @ 2011-12-18 5:07 ` Jeff King 2011-12-18 5:38 ` Jeff King 0 siblings, 1 reply; 4+ messages in thread From: Jeff King @ 2011-12-18 5:07 UTC (permalink / raw) To: Ryan O'Hara; +Cc: git, Junio C Hamano On Sun, Dec 18, 2011 at 12:03:22AM -0500, Jeff King wrote: > Subject: [PATCH] commit: initialize static strbuf > > Strbufs cannot rely on static all-zero initialization; > instead, they must use STRBUF_INIT to point to the > "slopbuf". > > Without this patch, "git commit --no-message" segfaults > reliably. This one, too: diff --git a/builtin/merge.c b/builtin/merge.c index 2457940..28a3a7e 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -50,7 +50,7 @@ struct strategy { static int fast_forward_only, option_edit; static int allow_trivial = 1, have_message; static int overwrite_ignore = 1; -static struct strbuf merge_msg; +static struct strbuf merge_msg = STRBUF_INIT; static struct commit_list *remoteheads; static struct strategy **use_strategies; static size_t use_strategies_nr, use_strategies_alloc; I'm not sure if you can actually trigger a segfault, but it clearly violates the strbuf API. -Peff ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: BUG: Git command causes crash 2011-12-18 5:07 ` Jeff King @ 2011-12-18 5:38 ` Jeff King 0 siblings, 0 replies; 4+ messages in thread From: Jeff King @ 2011-12-18 5:38 UTC (permalink / raw) To: Ryan O'Hara; +Cc: git, Junio C Hamano On Sun, Dec 18, 2011 at 12:07:28AM -0500, Jeff King wrote: > -static struct strbuf merge_msg; > +static struct strbuf merge_msg = STRBUF_INIT; > [...] > > I'm not sure if you can actually trigger a segfault, but it clearly > violates the strbuf API. One note on this: that line by itself doesn't violate the strbuf API. You can always initialize it by hand later. But it's easy to look through the code in this case and see that we don't. A grep of "static struct strbuf" doesn't show any other similar cases. I also did a grep of "struct strbuf [a-z]*;" to find stack and struct member variables. Most of the stack ones get initialized right away (which is not too surprising, as they would contain random garbage and not work at all). A few of the ones in structs are hard to track down, and could be problematic (I expected to maybe find a memset-to-zero initialization), but I followed all of them to an actual strbuf_init. Which isn't to say my looking was exhaustive, as there might be other code paths I missed. But at least these two are the low-hanging fruit. Here's a commit which does them both together (they are really the same bug). -- >8 -- Subject: [PATCH] initialize static strbufs Strbufs cannot rely on static all-zero initialization; instead, they must use STRBUF_INIT to point to the "slopbuf". This failure can go undetected in most code paths, as only some of the strbuf functions assume the slopbuf is in place (e.g., "strbuf_add" works). In particular, calling "strbuf_setlen" will cause a segfault. Without this patch, "git commit --no-message" and "git merge --no-message" segfault reliably. Signed-off-by: Jeff King <peff@peff.net> --- builtin/commit.c | 2 +- builtin/merge.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index d0f27f9..336faff 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -104,7 +104,7 @@ static int use_editor = 1, include_status = 1; static int show_ignored_in_status; static const char *only_include_assumed; -static struct strbuf message; +static struct strbuf message = STRBUF_INIT; static int null_termination; static enum { diff --git a/builtin/merge.c b/builtin/merge.c index 2457940..28a3a7e 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -50,7 +50,7 @@ struct strategy { static int fast_forward_only, option_edit; static int allow_trivial = 1, have_message; static int overwrite_ignore = 1; -static struct strbuf merge_msg; +static struct strbuf merge_msg = STRBUF_INIT; static struct commit_list *remoteheads; static struct strategy **use_strategies; static size_t use_strategies_nr, use_strategies_alloc; -- 1.7.8.rc3.14.gd2470 ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-12-18 5:38 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-12-18 3:13 BUG: Git command causes crash Ryan O'Hara 2011-12-18 5:03 ` Jeff King 2011-12-18 5:07 ` Jeff King 2011-12-18 5:38 ` Jeff King
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).