{"thread":{"id":"29200","subject":"BUG: Git command causes crash","startedAt":"2011-12-18T03:13:53Z","lastAt":"2011-12-18T05:38:04Z","messageCount":4,"participants":["Ryan O'Hara","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"181426","messageId":"CAOgd6zFr5LorTK6X5o6NQE3L61KhaUZG9tX4LEB4_Na_YKPPpA@mail.gmail.com","threadId":"29200","inReplyTo":null,"subject":"BUG: Git command causes crash","fromName":"Ryan O'Hara","fromEmail":"minitech.me@gmail.com","sentAt":"2011-12-18T03:13:53Z","receivedAt":"2011-12-18T03:13:53Z","isPatch":false,"sender":{"key":"minitech.me@gmail.com","avatar":null},"body":"On Git for Windows (MinGW), at least, this command causes git to crash:\n\ngit commit -a --no-message --dry-run\n"},{"id":"181428","messageId":"20111218050322.GA1787@sigill.intra.peff.net","threadId":"29200","inReplyTo":"CAOgd6zFr5LorTK6X5o6NQE3L61KhaUZG9tX4LEB4_Na_YKPPpA@mail.gmail.com","subject":"Re: BUG: Git command causes crash","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-12-18T05:03:22Z","receivedAt":"2011-12-18T05:03:22Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Dec 17, 2011 at 07:13:53PM -0800, Ryan O'Hara wrote:\n\n> On Git for Windows (MinGW), at least, this command causes git to crash:\n> \n> git commit -a --no-message --dry-run\n\nOn Linux, too, using just \"git commit --no-message\" (whether there is\nsomething to commit or not). This fixes it for me.\n\n-- >8 --\nSubject: [PATCH] commit: initialize static strbuf\n\nStrbufs cannot rely on static all-zero initialization;\ninstead, they must use STRBUF_INIT to point to the\n\"slopbuf\".\n\nWithout this patch, \"git commit --no-message\" segfaults\nreliably.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/commit.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex d0f27f9..336faff 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -104,7 +104,7 @@\n static int use_editor = 1, include_status = 1;\n static int show_ignored_in_status;\n static const char *only_include_assumed;\n-static struct strbuf message;\n+static struct strbuf message = STRBUF_INIT;\n \n static int null_termination;\n static enum {\n-- \n1.7.8.rc3.14.gd2470\n"},{"id":"181429","messageId":"20111218050728.GB1787@sigill.intra.peff.net","threadId":"29200","inReplyTo":"20111218050322.GA1787@sigill.intra.peff.net","subject":"Re: BUG: Git command causes crash","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-12-18T05:07:28Z","receivedAt":"2011-12-18T05:07:28Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Dec 18, 2011 at 12:03:22AM -0500, Jeff King wrote:\n\n> Subject: [PATCH] commit: initialize static strbuf\n> \n> Strbufs cannot rely on static all-zero initialization;\n> instead, they must use STRBUF_INIT to point to the\n> \"slopbuf\".\n> \n> Without this patch, \"git commit --no-message\" segfaults\n> reliably.\n\nThis one, too:\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 2457940..28a3a7e 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -50,7 +50,7 @@ struct strategy {\n static int fast_forward_only, option_edit;\n static int allow_trivial = 1, have_message;\n static int overwrite_ignore = 1;\n-static struct strbuf merge_msg;\n+static struct strbuf merge_msg = STRBUF_INIT;\n static struct commit_list *remoteheads;\n static struct strategy **use_strategies;\n static size_t use_strategies_nr, use_strategies_alloc;\n\nI'm not sure if you can actually trigger a segfault, but it clearly\nviolates the strbuf API.\n\n-Peff\n"},{"id":"181430","messageId":"20111218053804.GA4462@sigill.intra.peff.net","threadId":"29200","inReplyTo":"20111218050728.GB1787@sigill.intra.peff.net","subject":"Re: BUG: Git command causes crash","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-12-18T05:38:04Z","receivedAt":"2011-12-18T05:38:04Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Dec 18, 2011 at 12:07:28AM -0500, Jeff King wrote:\n\n> -static struct strbuf merge_msg;\n> +static struct strbuf merge_msg = STRBUF_INIT;\n> [...]\n> \n> I'm not sure if you can actually trigger a segfault, but it clearly\n> violates the strbuf API.\n\nOne note on this: that line by itself doesn't violate the strbuf API.\nYou can always initialize it by hand later.  But it's easy to look\nthrough the code in this case and see that we don't.\n\nA grep of \"static struct strbuf\" doesn't show any other similar cases. I\nalso did a grep of \"struct strbuf [a-z]*;\" to find stack and struct\nmember variables. Most of the stack ones get initialized right away\n(which is not too surprising, as they would contain random garbage and\nnot work at all). A few of the ones in structs are hard to track down,\nand could be problematic (I expected to maybe find a memset-to-zero\ninitialization), but I followed all of them to an actual strbuf_init.\n\nWhich isn't to say my looking was exhaustive, as there might be other\ncode paths I missed. But at least these two are the low-hanging fruit.\n\nHere's a commit which does them both together (they are really the same\nbug).\n\n-- >8 --\nSubject: [PATCH] initialize static strbufs\n\nStrbufs cannot rely on static all-zero initialization;\ninstead, they must use STRBUF_INIT to point to the\n\"slopbuf\". This failure can go undetected in most code\npaths, as only some of the strbuf functions assume the\nslopbuf is in place (e.g., \"strbuf_add\" works). In\nparticular, calling \"strbuf_setlen\" will cause a segfault.\n\nWithout this patch, \"git commit --no-message\" and \"git merge\n--no-message\" segfault reliably.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/commit.c |    2 +-\n builtin/merge.c  |    2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex d0f27f9..336faff 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -104,7 +104,7 @@\n static int use_editor = 1, include_status = 1;\n static int show_ignored_in_status;\n static const char *only_include_assumed;\n-static struct strbuf message;\n+static struct strbuf message = STRBUF_INIT;\n \n static int null_termination;\n static enum {\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 2457940..28a3a7e 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -50,7 +50,7 @@ struct strategy {\n static int fast_forward_only, option_edit;\n static int allow_trivial = 1, have_message;\n static int overwrite_ignore = 1;\n-static struct strbuf merge_msg;\n+static struct strbuf merge_msg = STRBUF_INIT;\n static struct commit_list *remoteheads;\n static struct strategy **use_strategies;\n static size_t use_strategies_nr, use_strategies_alloc;\n-- \n1.7.8.rc3.14.gd2470\n"}]}