{"thread":{"id":"39655","subject":"[PATCH] strbuf: stop out-of-boundary warnings from Coverity","startedAt":"2015-06-17T10:16:49Z","lastAt":"2015-06-19T15:27:42Z","messageCount":13,"participants":["Nguyễn Thái Ngọc Duy","Stefan Beller","Jeff King","Junio C Hamano","Duy Nguyen","Remi Galan Alfonso"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"264000","messageId":"1434536209-31350-1-git-send-email-pclouds@gmail.com","threadId":"39655","inReplyTo":null,"subject":"[PATCH] strbuf: stop out-of-boundary warnings from Coverity","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2015-06-17T10:16:49Z","receivedAt":"2015-06-17T10:16:49Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"It usually goes like this\n\n strbuf sb = STRBUF_INIT;\n if (!strncmp(sb.buf, \"foo\", 3))\n printf(\"%s\", sb.buf + 3);\n\nCoverity thinks that printf() can be executed, and because initial\nsb.buf only has one character (from strbuf_slopbuf), sb.buf + 3 is out\nof bound. What it does not recognize is strbuf_slopbuf[0] is always (*)\nzero. We always do some string comparison before jumping ahead to\n\"sb.buf + 3\" and those operations will stop out of bound accesses.\n\nJust make strbuf_slopbuf[] large enough to keep Coverity happy. If it's\nhappy, we'll have cleaner defect list and better chances of spotting\ntrue defects.\n\n(*) It's not entirely wrong though. Somebody may do sb.buf[0] = 'f'\n right after variable declaration and ruin all unused strbuf.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n There are lots of false warnings like this from Coverity. I just\n wanted to kill them off so we can spot more serious problems easier.\n I can't really verify that this patch shuts off those warnings\n because scan.coverity.com policy does not allow forks.\n\n I had another patch that avoids corrupting strbuf_slopbuf, by putting\n it to .rodata section. The patch is more invasive though, because\n this statement buf.buf[buf.len] = '\\0' now has to make sure buf.buf\n is not strbuf_slopbuf. It feels safer, but probably not enough to\n justify the change.\n\n strbuf.c | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/strbuf.c b/strbuf.c\nindex 0d4f4e5..0d7c3cf 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -16,7 +16,12 @@ int starts_with(const char *str, const char *prefix)\n * buf is non NULL and ->buf is NUL terminated even for a freshly\n * initialized strbuf.\n */\n+#ifndef __COVERITY__\n char strbuf_slopbuf[1];\n+#else\n+/* Stop so many incorrect out-of-boundary warnings from Coverity */\n+char strbuf_slopbuf[64];\n+#endif\n \n void strbuf_init(struct strbuf *sb, size_t hint)\n {\n-- \n2.3.0.rc1.137.g477eb31\n"},{"id":"264049","messageId":"CAGZ79kYRfjeXGkYAv-Kn2Bk-pp2ZSzpKGHDhqMpw03scdRZAmQ@mail.gmail.com","threadId":"39655","inReplyTo":"1434536209-31350-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] strbuf: stop out-of-boundary warnings from Coverity","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-17T17:28:15Z","receivedAt":"2015-06-17T17:28:15Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Jun 17, 2015 at 3:16 AM, Nguyễn Thái Ngọc Duy wrote:\n> It usually goes like this\n>\n> strbuf sb = STRBUF_INIT;\n> if (!strncmp(sb.buf, \"foo\", 3))\n> printf(\"%s\", sb.buf + 3);\n>\n> Coverity thinks that printf() can be executed, and because initial\n> sb.buf only has one character (from strbuf_slopbuf), sb.buf + 3 is out\n> of bound. What it does not recognize is strbuf_slopbuf[0] is always (*)\n> zero. We always do some string comparison before jumping ahead to\n> \"sb.buf + 3\" and those operations will stop out of bound accesses.\n>\n> Just make strbuf_slopbuf[] large enough to keep Coverity happy. If it's\n> happy, we'll have cleaner defect list and better chances of spotting\n> true defects.\n>\n> (*) It's not entirely wrong though. Somebody may do sb.buf[0] = 'f'\n> right after variable declaration and ruin all unused strbuf.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> There are lots of false warnings like this from Coverity. I just\n> wanted to kill them off so we can spot more serious problems easier.\n> I can't really verify that this patch shuts off those warnings\n> because scan.coverity.com policy does not allow forks.\n\nThanks a lot for looking into this!\nI'll just took this patch and have run a custom build now.\n(Depending on the outcome of the discussion, I may just carry\nthis patch around on top of the origin/pu for each scan.)\n\nThis patch is however a work around and not fixing the root problem.\n(The root problem being, coverity is not good enough to understand the\nimplications of strncmp, skip_prefix, starts_with or such).\n\nThe trade off is we're not able to detect problems with strbuf if any arise.\n\n\n>\n> I had another patch that avoids corrupting strbuf_slopbuf, by putting\n> it to .rodata section. The patch is more invasive though, because\n> this statement buf.buf[buf.len] = '\\0' now has to make sure buf.buf\n> is not strbuf_slopbuf. It feels safer, but probably not enough to\n> justify the change.\n>\n> strbuf.c | 5 +++++\n> 1 file changed, 5 insertions(+)\n>\n> diff --git a/strbuf.c b/strbuf.c\n> index 0d4f4e5..0d7c3cf 100644\n> --- a/strbuf.c\n> +++ b/strbuf.c\n> @@ -16,7 +16,12 @@ int starts_with(const char *str, const char *prefix)\n> * buf is non NULL and ->buf is NUL terminated even for a freshly\n> * initialized strbuf.\n> */\n> +#ifndef __COVERITY__\n> char strbuf_slopbuf[1];\n> +#else\n> +/* Stop so many incorrect out-of-boundary warnings from Coverity */\n> +char strbuf_slopbuf[64];\n> +#endif\n>\n> void strbuf_init(struct strbuf *sb, size_t hint)\n> {\n> --\n> 2.3.0.rc1.137.g477eb31\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"264052","messageId":"CAGZ79kbZpiz2rMbhJReFG=uRiQdj7a5qxLbRiPQQCFqcfBhikw@mail.gmail.com","threadId":"39655","inReplyTo":"CAGZ79kYRfjeXGkYAv-Kn2Bk-pp2ZSzpKGHDhqMpw03scdRZAmQ@mail.gmail.com","subject":"Re: [PATCH] strbuf: stop out-of-boundary warnings from Coverity","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-17T17:58:10Z","receivedAt":"2015-06-17T17:58:10Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"> Just make strbuf_slopbuf[] large enough to keep Coverity happy. If it's\n> happy, we'll have cleaner defect list\n\nIt's down 31 defects, roughly 10% of all things coverity detected as\nproblematic.\nYAY!\n"},{"id":"264056","messageId":"20150617191235.GB25304@peff.net","threadId":"39655","inReplyTo":"CAGZ79kbZpiz2rMbhJReFG=uRiQdj7a5qxLbRiPQQCFqcfBhikw@mail.gmail.com","subject":"Re: [PATCH] strbuf: stop out-of-boundary warnings from Coverity","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-06-17T19:12:35Z","receivedAt":"2015-06-17T19:12:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 17, 2015 at 10:58:10AM -0700, Stefan Beller wrote:\n\n> > Just make strbuf_slopbuf[] large enough to keep Coverity happy. If it's\n> > happy, we'll have cleaner defect list\n> \n> It's down 31 defects, roughly 10% of all things coverity detected as\n> problematic.\n> YAY!\n\nThat's a good thing. I do find the solution a little gross, though. I\nwonder if there is a way we can tell coverity more about how strbuf\nworks. I've noticed similar problems with string_list, where it\ncomplains that we are touching list->items, which was assigned to NULL\n(of course it was, but then after that we did string_list_append!).\n\nI know literally nothing about coverity's annotations and what's\npossible with them. So I may be barking up a wrong tree completely.\n\n-Peff\n"},{"id":"264064","messageId":"xmqqh9q66ss9.fsf@gitster.dls.corp.google.com","threadId":"39655","inReplyTo":"CAGZ79kbZpiz2rMbhJReFG=uRiQdj7a5qxLbRiPQQCFqcfBhikw@mail.gmail.com","subject":"Re: [PATCH] strbuf: stop out-of-boundary warnings from Coverity","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-17T19:25:58Z","receivedAt":"2015-06-17T19:25:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n>> Just make strbuf_slopbuf[] large enough to keep Coverity happy. If it's\n>> happy, we'll have cleaner defect list\n>\n> It's down 31 defects, roughly 10% of all things coverity detected as\n> problematic.\n> YAY!\n\nI actually think this is too ugly to live. If coverity is buggy and\nunusable, why aren't we raising that issue to them?\n"},{"id":"264078","messageId":"CAGZ79kZCGck24KZBhVa9TGtR4U6Moz-vHNrBMk0HLTz6YeHhsg@mail.gmail.com","threadId":"39655","inReplyTo":"20150617191235.GB25304@peff.net","subject":"Re: [PATCH] strbuf: stop out-of-boundary warnings from Coverity","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-17T20:03:03Z","receivedAt":"2015-06-17T20:03:03Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Jun 17, 2015 at 12:12 PM, Jeff King wrote:\n> On Wed, Jun 17, 2015 at 10:58:10AM -0700, Stefan Beller wrote:\n>\n>> > Just make strbuf_slopbuf[] large enough to keep Coverity happy. If it's\n>> > happy, we'll have cleaner defect list\n>>\n>> It's down 31 defects, roughly 10% of all things coverity detected as\n>> problematic.\n>> YAY!\n>\n> That's a good thing. I do find the solution a little gross, though. I\n> wonder if there is a way we can tell coverity more about how strbuf\n> works.\n\nI always thought the problem was a combination of both having a custom\nstrcmp (like skip_prefix, starts_with) and a custom data structure (strbuf,\nstring_list). So I am not sure if it is sufficient to tell coverity\n\n> I've noticed similar problems with string_list, where it\n> complains that we are touching list->items, which was assigned to NULL\n> (of course it was, but then after that we did string_list_append!).\n>\n> I know literally nothing about coverity's annotations and what's\n> possible with them. So I may be barking up a wrong tree completely.\n\nI have searched for the exact annotations for a while, but all I found\nwere examples in other open source projects, no official documentation\nwith all its features. I may be missing something though (there must be\nsome official documentation, I'd assume).\n\n>\n> -Peff\n"},{"id":"264079","messageId":"CAGZ79kbwXGkBJrX793Ry1gn_5uBymrXWz=yy00WDADNjG1m-=A@mail.gmail.com","threadId":"39655","inReplyTo":"xmqqh9q66ss9.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] strbuf: stop out-of-boundary warnings from Coverity","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-17T20:05:05Z","receivedAt":"2015-06-17T20:05:05Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Jun 17, 2015 at 12:25 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>>> Just make strbuf_slopbuf[] large enough to keep Coverity happy. If it's\n>>> happy, we'll have cleaner defect list\n>>\n>> It's down 31 defects, roughly 10% of all things coverity detected as\n>> problematic.\n>> YAY!\n>\n> I actually think this is too ugly to live. If coverity is buggy and\n> unusable, why aren't we raising that issue to them?\n\nWe can try to do that.\nThe last time I wanted them to take a look at Git, they were\nunresponsive. I presume that's what you get when not being\na paying customer. :(\n"},{"id":"264114","messageId":"20150618101353.GA6525@lanh","threadId":"39655","inReplyTo":"20150617191235.GB25304@peff.net","subject":"Re: [PATCH] strbuf: stop out-of-boundary warnings from Coverity","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-06-18T10:13:53Z","receivedAt":"2015-06-18T10:13:53Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Jun 17, 2015 at 03:12:35PM -0400, Jeff King wrote:\n> On Wed, Jun 17, 2015 at 10:58:10AM -0700, Stefan Beller wrote:\n> \n> > > Just make strbuf_slopbuf[] large enough to keep Coverity happy. If it's\n> > > happy, we'll have cleaner defect list\n> > \n> > It's down 31 defects, roughly 10% of all things coverity detected as\n> > problematic.\n> > YAY!\n> \n> That's a good thing. I do find the solution a little gross, though. I\n> wonder if there is a way we can tell coverity more about how strbuf\n> works. I've noticed similar problems with string_list, where it\n> complains that we are touching list->items, which was assigned to NULL\n> (of course it was, but then after that we did string_list_append!).\n\nThere's \"function modeling\" where you write simplified (and likely\nincorrect) version of a function to correct how coverity's\nunderstanding of that function. I searched, there's no \"data\nmodeling\". I think I have the user manual, but haven't looked through\nit yet.\n\nOn Thu, Jun 18, 2015 at 2:25 AM, Junio C Hamano wrote:\n> I actually think this is too ugly to live.\n\nWell, I have another option below. Let's see how people feel about it.\n\n> If coverity is buggy and unusable, why aren't we raising that issue\n> to them?\n\nIt's technically correct though. The key point here is strbuf_slopbuf[0]\nis NUL, but that cannot be statically determined. And we may also need\nto teach it about strcmp' and friends' semantics. That's probably too\nmuch for a static analyzer.\n\nThe last resort is simply filter out a whole class of warnings.\nProbably good enough if both patches look equally ugly.\n\n-- 8< --\nSubject: [PATCH] strbuf: kill strbuf_slopbuf, in favor of \"\"\n\nA lot of \"out-of-bound access\" warnings on scan.coverity.com is because\nit does not realize this strbuf_slopbuf[] is in fact initialized with a\nsingle and promised to never change. But that promise could be broken if\nsome caller attempts to write to strbuf->buf[0] write after STRBUF_INIT.\n\nWe really can't do much about it. But we can try to put strbuf_slopbuf\nin .rodata section, where writes will be caught by the OS with memory\nprotection support. The only drawback is people can't do\n\"buf->buf == strbuf_slopbuf\" any more. Luckily nobody does that in the\ncurrent code base.\n---\n strbuf.c | 19 ++++++-------------\n strbuf.h | 11 ++++++++---\n 2 files changed, 14 insertions(+), 16 deletions(-)\n\ndiff --git a/strbuf.c b/strbuf.c\nindex 0d4f4e5..9f91229 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -11,17 +11,10 @@ int starts_with(const char *str, const char *prefix)\n \t\t\treturn 0;\n }\n \n-/*\n- * Used as the default ->buf value, so that people can always assume\n- * buf is non NULL and ->buf is NUL terminated even for a freshly\n- * initialized strbuf.\n- */\n-char strbuf_slopbuf[1];\n-\n void strbuf_init(struct strbuf *sb, size_t hint)\n {\n \tsb->alloc = sb->len = 0;\n-\tsb->buf = strbuf_slopbuf;\n+\tsb->buf = (char*)\"\";\n \tif (hint)\n \t\tstrbuf_grow(sb, hint);\n }\n@@ -52,7 +45,7 @@ void strbuf_attach(struct strbuf *sb, void *buf, size_t len, size_t alloc)\n \tsb->len = len;\n \tsb->alloc = alloc;\n \tstrbuf_grow(sb, 0);\n-\tsb->buf[sb->len] = '\\0';\n+\tstrbuf_terminate(sb);\n }\n \n void strbuf_grow(struct strbuf *sb, size_t extra)\n@@ -77,7 +70,7 @@ void strbuf_rtrim(struct strbuf *sb)\n {\n \twhile (sb->len > 0 && isspace((unsigned char)sb->buf[sb->len - 1]))\n \t\tsb->len--;\n-\tsb->buf[sb->len] = '\\0';\n+\tstrbuf_terminate(sb);\n }\n \n void strbuf_ltrim(struct strbuf *sb)\n@@ -88,7 +81,7 @@ void strbuf_ltrim(struct strbuf *sb)\n \t\tsb->len--;\n \t}\n \tmemmove(sb->buf, b, sb->len);\n-\tsb->buf[sb->len] = '\\0';\n+\tstrbuf_terminate(sb);\n }\n \n int strbuf_reencode(struct strbuf *sb, const char *from, const char *to)\n@@ -380,7 +373,7 @@ ssize_t strbuf_read(struct strbuf *sb, int fd, size_t hint)\n \t\tstrbuf_grow(sb, 8192);\n \t}\n \n-\tsb->buf[sb->len] = '\\0';\n+\tstrbuf_terminate(sb);\n \treturn sb->len - oldlen;\n }\n \n@@ -496,7 +489,7 @@ int strbuf_getwholeline(struct strbuf *sb, FILE *fp, int term)\n \tif (ch == EOF && sb->len == 0)\n \t\treturn EOF;\n \n-\tsb->buf[sb->len] = '\\0';\n+\tstrbuf_terminate(sb);\n \treturn 0;\n }\n #endif\ndiff --git a/strbuf.h b/strbuf.h\nindex 01c5c63..d8346ee 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -67,8 +67,13 @@ struct strbuf {\n \tchar *buf;\n };\n \n-extern char strbuf_slopbuf[];\n-#define STRBUF_INIT { 0, 0, strbuf_slopbuf }\n+#define STRBUF_INIT { 0, 0, (char*)\"\" }\n+\n+static inline void strbuf_terminate(struct strbuf *sb)\n+{\n+\tif (sb->buf[sb->len])\n+\t\tsb->buf[sb->len] = '\\0';\n+}\n \n /**\n * Life Cycle Functions\n@@ -149,7 +154,7 @@ static inline void strbuf_setlen(struct strbuf *sb, size_t len)\n \tif (len > (sb->alloc ? sb->alloc - 1 : 0))\n \t\tdie(\"BUG: strbuf_setlen() beyond buffer\");\n \tsb->len = len;\n-\tsb->buf[len] = '\\0';\n+\tstrbuf_terminate(sb);\n }\n \n /**\n-- \n2.3.0.rc1.137.g477eb31\n-- 8< --\n-- \nDuy\n"},{"id":"264200","messageId":"xmqqioalezhq.fsf@gitster.dls.corp.google.com","threadId":"39655","inReplyTo":"20150618101353.GA6525@lanh","subject":"Re: [PATCH] strbuf: stop out-of-boundary warnings from Coverity","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-18T16:46:09Z","receivedAt":"2015-06-18T16:46:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> The last resort is simply filter out a whole class of warnings.\n> Probably good enough if both patches look equally ugly.\n>\n> -- 8< --\n> Subject: [PATCH] strbuf: kill strbuf_slopbuf, in favor of \"\"\n>\n> A lot of \"out-of-bound access\" warnings on scan.coverity.com is because\n> it does not realize this strbuf_slopbuf[] is in fact initialized with a\n> single and promised to never change. But that promise could be broken if\n> some caller attempts to write to strbuf->buf[0] write after STRBUF_INIT.\n>\n> We really can't do much about it. But we can try to put strbuf_slopbuf\n> in .rodata section, where writes will be caught by the OS with memory\n> protection support. The only drawback is people can't do\n> \"buf->buf == strbuf_slopbuf\" any more. Luckily nobody does that in the\n> current code base.\n> ---\n\nHmph, would declaring slopbuf as \"const char [1]\" (and sprinkling\nthe \"(char *)\" cast) have the same effect, I wonder?\n\n> +static inline void strbuf_terminate(struct strbuf *sb)\n> +{\n> +\tif (sb->buf[sb->len])\n> +\t\tsb->buf[sb->len] = '\\0';\n> +}\n\nThis is so that you can call things like strbuf_rtrim() immediately\nafter running strbuf_init() safely, but I think it needs a comment\nto save people from wondering what is going on, e.g. \"this is not an\noptimization to avoid assigning NUL to a place that is already NUL;\na freshly initialized strbuf points at an unwritable piece of NUL\nand we do not want to cause a SEGV\".\n"},{"id":"264294","messageId":"20150619103946.GA20493@lanh","threadId":"39655","inReplyTo":"xmqqioalezhq.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] strbuf: stop out-of-boundary warnings from Coverity","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-06-19T10:39:46Z","receivedAt":"2015-06-19T10:39:46Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Jun 18, 2015 at 09:46:09AM -0700, Junio C Hamano wrote:\n> Duy Nguyen writes:\n> \n> > The last resort is simply filter out a whole class of warnings.\n> > Probably good enough if both patches look equally ugly.\n> >\n> > -- 8< --\n> > Subject: [PATCH] strbuf: kill strbuf_slopbuf, in favor of \"\"\n> >\n> > A lot of \"out-of-bound access\" warnings on scan.coverity.com is because\n> > it does not realize this strbuf_slopbuf[] is in fact initialized with a\n> > single and promised to never change. But that promise could be broken if\n> > some caller attempts to write to strbuf->buf[0] write after STRBUF_INIT.\n> >\n> > We really can't do much about it. But we can try to put strbuf_slopbuf\n> > in .rodata section, where writes will be caught by the OS with memory\n> > protection support. The only drawback is people can't do\n> > \"buf->buf == strbuf_slopbuf\" any more. Luckily nobody does that in the\n> > current code base.\n> > ---\n> \n> Hmph, would declaring slopbuf as \"const char [1]\" (and sprinkling\n> the \"(char *)\" cast) have the same effect, I wonder?\n\nI remember I tried that and failed with a compile error. I just tried\nagain, this time it worked. Must have made some stupid mistake in my\nfirst try.\n\nAnyway it does not put strbuf_slopbuf in .rodata. \"./git\" does not\nsegfault with this patch\n\n-- 8< --\ndiff --git a/git.c b/git.c\nindex 44374b1..960e375 100644\n--- a/git.c\n+++ b/git.c\n@@ -621,7 +621,11 @@ int main(int argc, char **av)\n \tconst char **argv = (const char **) av;\n \tconst char *cmd;\n \tint done_help = 0;\n+\tstruct strbuf sb = STRBUF_INIT;\n \n+\tsb.buf[0] = 'Z';\n+\tprintf(\"%c\\n\", strbuf_slopbuf[0]);\n+\treturn 0;\n \tstartup_info = &git_startup_info;\n \n \tcmd = git_extract_argv0_path(argv[0]);\ndiff --git a/strbuf.c b/strbuf.c\nindex 0d4f4e5..3a0d4c9 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -16,12 +16,12 @@ int starts_with(const char *str, const char *prefix)\n * buf is non NULL and ->buf is NUL terminated even for a freshly\n * initialized strbuf.\n */\n-char strbuf_slopbuf[1];\n+const char strbuf_slopbuf[1];\n \n void strbuf_init(struct strbuf *sb, size_t hint)\n {\n \tsb->alloc = sb->len = 0;\n-\tsb->buf = strbuf_slopbuf;\n+\tsb->buf = (char *)strbuf_slopbuf;\n \tif (hint)\n \t\tstrbuf_grow(sb, hint);\n }\ndiff --git a/strbuf.h b/strbuf.h\nindex 01c5c63..eab7307 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -67,8 +67,8 @@ struct strbuf {\n \tchar *buf;\n };\n \n-extern char strbuf_slopbuf[];\n-#define STRBUF_INIT { 0, 0, strbuf_slopbuf }\n+extern const char strbuf_slopbuf[];\n+#define STRBUF_INIT { 0, 0, (char *)strbuf_slopbuf }\n \n /**\n * Life Cycle Functions\n-- 8< --\n\n> > +static inline void strbuf_terminate(struct strbuf *sb)\n> > +{\n> > +\tif (sb->buf[sb->len])\n> > +\t\tsb->buf[sb->len] = '\\0';\n> > +}\n> \n> This is so that you can call things like strbuf_rtrim() immediately\n> after running strbuf_init() safely, but I think it needs a comment\n> to save people from wondering what is going on, e.g. \"this is not an\n> optimization to avoid assigning NUL to a place that is already NUL;\n> a freshly initialized strbuf points at an unwritable piece of NUL\n> and we do not want to cause a SEGV\".\n\nSure, if we go with this direction.\n--\nDuy\n"},{"id":"264296","messageId":"319721281.628788.1434711025325.JavaMail.zimbra@ensimag.grenoble-inp.fr","threadId":"39655","inReplyTo":"20150619103946.GA20493@lanh","subject":"Re: [PATCH] strbuf: stop out-of-boundary warnings from Coverity","fromName":"Remi Galan Alfonso","fromEmail":"remi.galan-alfonso@ensimag.grenoble-inp.fr","sentAt":"2015-06-19T10:50:25Z","receivedAt":"2015-06-19T10:50:25Z","isPatch":true,"sender":{"key":"remi.galan-alfonso@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/12509162?v=4"},"body":"Duy Nguyen writes:\n\n> + sb.buf[0] = 'Z';\n> + printf(\"%c\\n\", strbuf_slopbuf[0]);\n> + return 0;\n> startup_info = &git_startup_info;\n\nI might be wrong, but I definitely think that this\nprintf and return 0 are some debug lines that you \nforgot to remove.\n\nRémi\n"},{"id":"264298","messageId":"CACsJy8AJdjh_XjdEhhykBDMRPogMcV3LLCQpR2o1T3P1GkVKfw@mail.gmail.com","threadId":"39655","inReplyTo":"319721281.628788.1434711025325.JavaMail.zimbra@ensimag.grenoble-inp.fr","subject":"Re: [PATCH] strbuf: stop out-of-boundary warnings from Coverity","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-06-19T10:51:32Z","receivedAt":"2015-06-19T10:51:32Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Jun 19, 2015 at 5:50 PM, Remi Galan Alfonso\n wrote:\n> Duy Nguyen writes:\n>\n>> + sb.buf[0] = 'Z';\n>> + printf(\"%c\\n\", strbuf_slopbuf[0]);\n>> + return 0;\n>> startup_info = &git_startup_info;\n>\n> I might be wrong, but I definitely think that this\n> printf and return 0 are some debug lines that you\n> forgot to remove.\n\nYes it's debug code. I hoped that if I made a mistake forcing the\nsegfault, people would notice.\n-- \nDuy\n"},{"id":"264334","messageId":"xmqqegl7btw1.fsf@gitster.dls.corp.google.com","threadId":"39655","inReplyTo":"20150619103946.GA20493@lanh","subject":"Re: [PATCH] strbuf: stop out-of-boundary warnings from Coverity","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-19T15:27:42Z","receivedAt":"2015-06-19T15:27:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> Anyway it does not put strbuf_slopbuf in .rodata.\n\nThat's sad. I wa hoping that it would behave the same as this,\nwhich does give me SEGV:\n\n#include \n\nstatic const char x = '\\0';\nstatic char *y = (char *)&x;\n\nint main (void) {\n\t*y = 1;\n}\n"}]}