<div dir="ltr">On Tue, Sep 10, 2013 at 2:50 PM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On Tue, Sep 10, 2013 at 5:40 PM, Eli Friedman <<a href="mailto:eli.friedman@gmail.com">eli.friedman@gmail.com</a>> wrote:<br>

> On Tue, Sep 10, 2013 at 2:33 PM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>><br>
> wrote:<br>
>><br>
>> On Tue, Sep 10, 2013 at 5:21 PM, Eli Friedman <<a href="mailto:eli.friedman@gmail.com">eli.friedman@gmail.com</a>><br>
>> wrote:<br>
>> > On Tue, Sep 10, 2013 at 2:03 PM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>><br>
>> > wrote:<br>
>> >><br>
>> >> Currently, if you have a value assignment involving structures<br>
>> >> containing a flexible array, no diagnostic is emitted.  However, since<br>
>> >> the array members won't be copied as part of the assignment, a<br>
>> >> diagnostic will help programmers avoid bugs.  This patch emits said<br>
>> >> diagnostic.<br>
>> >><br>
>> ><br>
>> > +def warn_flexible_array_assignment : ExtWarn<<br>
>> > +  "assignment of flexible arrays does not copy the array members">,<br>
>> > +  InGroup<FlexibleArrayExtensions>;<br>
>> ><br>
>> > I assume this is supposed to be a Warning, not an ExtWarn?<br>
>><br>
>> Yes, thanks for pointing that out.<br>
>><br>
>> > Also, I'm not sure putting this into the FlexibleArrayExtensions<br>
>> > diagnostic<br>
>> > group is the best idea.<br>
>><br>
>> I wasn't certain either, but it was the closest existing group.  Would<br>
>> it make more sense to add a new group for it?<br>
><br>
><br>
> I think so.<br>
<br>
</div>Any complaints with naming the group flexible-arrays?<br></blockquote><div><br></div><div>That's sort of vague; maybe flexible-array-slicing?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<div class="im"><br>
><br>
>><br>
>> > What sort of mistakes do you expect this to catch in practice?<br>
>><br>
>> Simple programmer mistakes, mostly.  It also nicely covers a CERT<br>
>> secure C coding rule:<br>
>><br>
>><br>
>> <a href="https://www.securecoding.cert.org/confluence/display/seccode/MEM33-C.++Allocate+and+copy+structures+containing+flexible+array+members+dynamically" target="_blank">https://www.securecoding.cert.org/confluence/display/seccode/MEM33-C.++Allocate+and+copy+structures+containing+flexible+array+members+dynamically</a><br>

><br>
><br>
> Okay.<br>
><br>
> Do you have any idea what the false positive rate is?<br>
<br>
</div>I would expect it to be quite low, but have nothing concrete to back<br>
that up.  If the warning turns out to be chatty for some reason, it<br>
could be tweaked easily enough (esp with a separate diagnostic group).<br></blockquote><div><br></div><div>Okay.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<div class="im">><br>
>><br>
>><br>
>><br>
>> > Have you considered warning about the variable declaration rather than<br>
>> > the<br>
>> > assignment?  In your testcase, it's impossible to call foo() without<br>
>> > triggering the warning, so it seems better to warn about foo() itself<br>
>> > rather<br>
>> > than the call.<br>
>><br>
>> Hmmm, would there ever be a case where it would make sense to declare<br>
>> a structure with a flexible array member as a value type?The only<br>
>><br>
>> situation I could think of would be overlaying the value type with<br>
>> stack-allocated memory in some sort of bizarre union type punning<br>
>> scenario.  So I'm thinking that may be a better approach than checking<br>
>> on assignment, unless there are intermediary ways you could get this<br>
>> assignment to happen in C.<br>
>><br>
><br>
> Well, you can write something like "*a = *b;", which doesn't involve<br>
> declaring a variable...<br>
<br>
</div>True.  Assignment would be the catch-all way to do it, since that's<br>
what causes the slicing behavior.  Perhaps both declaration and<br>
assignment diagnostics would make sense?  Declarations like that are<br>
definitively bizarre, but assignment still misbehaves.<br><br></blockquote><div><br></div><div>The CERT page classifies the mistakes into three categories; covering all of them seems reasonable.</div><div><br></div><div>
-Eli </div></div><br></div></div>