I haven't had a chance to look at the patch in detail, but had a few high-level comments.<br><br><div class="gmail_quote">On Mon, Dec 5, 2011 at 9:08 AM, Ingo Walther <span dir="ltr"><<a href="mailto:ingow@google.com">ingow@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div>The second pattern the patch catches is:</div><div>std::string v("abc", 10);</div><div><br></div><div>The constructor will read 10 characters into the new string, but there are only four (including the null terminator). This reads junk if we're lucky, and segfaults or opens security holes if we're unlucky.</div>
</blockquote><div><br></div><div>This is a really cool warning, but I wonder if we shouldn't distance it a bit from the first. Technically, this isn't constrained to the string constructor but to any function which accepts a pointer and a size. I could envision a nice C++11 annotation for the size parameter which refers to the pointer parameter clarifying that this is the expected size. Then anyone could write and annotate such interfaces and get warnings.</div>
<div><br></div><div>I'm totally happy to go forward with the current version and the current flag, but I'd like to leave some bread crumbs for the future when this can become a generic part of -Warray-bounds enabled via attributes on parameters to form the association.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">

<div>The new warning says:</div><div><div>str.cc:9:17: warning: string literal is shorter than the number of characters to read (3 vs 10) [-Wstring-constructor]</div></div></blockquote><div><br></div><div>A few nit-picks on the wording here. First, it seems like your patch (at a very distant glance) handles cases other than an explicit string literal. I wonder if it would be better to say 'C-string constant' or something else a touch more generic.</div>
<div><br></div><div>Second, I have trouble re-associating the '3' with the string literal's length and the 10 with the characters read. I wonder about putting the values directly into the message and reversing it.</div>
<div><br></div><div>"warning: constructor call reads 10 characters going 7 past the end of the 3 character string literal"</div><div><br></div><div>Meh, I don't really like it yet, but something in that direction I think would help me.</div>
<div> </div><div>Finally, I would put the location of the warning on the size rather than the string.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div><div>  std::string v("abc", 10);</div>

<div>                ^~~~~  ~~</div></div><div><br></div><div>If the literal is just one character long, we suggest to swap the arguments and turn the string literal into a char literal:</div><div><div>str.cc:8:17: warning: string literal is shorter than the number of characters to read (1 vs 10) [-Wstring-constructor]</div>


<div>  std::string u("a", 10);</div><div>                ^~~  ~~</div><div>str.cc:8:17: note: did you mean to swap the two arguments?</div><div>  std::string u("a", 10);</div><div>                ^~~~~~~</div>


<div>                10, 'a'</div></div></blockquote><div><br></div><div>Probably want the note to also mention switching from a string literal to a character literal.</div></div>