On Tue, Sep 10, 2013 at 5:21 PM, Eli Friedman <[email protected]> wrote: > On Tue, Sep 10, 2013 at 2:03 PM, Aaron Ballman <[email protected]> > wrote: >> >> Currently, if you have a value assignment involving structures >> containing a flexible array, no diagnostic is emitted. However, since >> the array members won't be copied as part of the assignment, a >> diagnostic will help programmers avoid bugs. This patch emits said >> diagnostic. >> > > +def warn_flexible_array_assignment : ExtWarn< > + "assignment of flexible arrays does not copy the array members">, > + InGroup<FlexibleArrayExtensions>; > > I assume this is supposed to be a Warning, not an ExtWarn?
Yes, thanks for pointing that out. > Also, I'm not sure putting this into the FlexibleArrayExtensions diagnostic > group is the best idea. I wasn't certain either, but it was the closest existing group. Would it make more sense to add a new group for it? > What sort of mistakes do you expect this to catch in practice? Simple programmer mistakes, mostly. It also nicely covers a CERT secure C coding rule: https://www.securecoding.cert.org/confluence/display/seccode/MEM33-C.++Allocate+and+copy+structures+containing+flexible+array+members+dynamically > Have you considered warning about the variable declaration rather than the > assignment? In your testcase, it's impossible to call foo() without > triggering the warning, so it seems better to warn about foo() itself rather > than the call. Hmmm, would there ever be a case where it would make sense to declare a structure with a flexible array member as a value type? The only situation I could think of would be overlaying the value type with stack-allocated memory in some sort of bizarre union type punning scenario. So I'm thinking that may be a better approach than checking on assignment, unless there are intermediary ways you could get this assignment to happen in C. Thanks! ~Aaron _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
