On Thu, Oct 08, 2020 at 08:36:02AM +0200, Ilya Maximets wrote:
> On 10/8/20 1:09 AM, Luc Van Oostenryck wrote:
> > On Wed, Oct 07, 2020 at 01:52:34PM +0200, Ilya Maximets wrote:
> >> There is a common pattern on how to allocate memory for a flexible-size
> >> structure, e.g.
> >>
> >> union {
> >> struct flex f; /* Structure that contains a flexible array. */
> >> char buf[MAX_SIZE]; /* Memeory buffer for structure 'flex' and
> >> its flexible array. */
> >> };
> >>
> >> There is another exmaple of such thing in CMSG manpage with the
> >> alignment purposes:
> >>
> >> union { /* Ancillary data buffer, wrapped in a union
> >> in order to ensure it is suitably aligned */
> >> char buf[CMSG_SPACE(sizeof(myfds))];
> >> struct cmsghdr align;
> >> } u;
> >>
> >> Such unions could form an array in case user wants multiple
> >> instances of them. For example, if you want receive up to
> >> 32 network packets via recvmmsg(), you will need 32 unions like 'u'.
> >> Open vSwitch does exactly that and fails the check.
> >>
> >> Disabling this check by default for unions. Adding new option
> >> -Wflex-union-array to enable it back. This option works only
> >> if -Wflex-array-array enabled (which is default behavior).
> >>
> >> Signed-off-by: Ilya Maximets <[email protected]>
> >> ---
> >>
> >> Not sure if this is a best way to fix the issue, but it works fine for
> >> openvswitch project. The actual code in question that makes sparse fail
> >> OVS build could be found here:
> >>
> >> https://github.com/openvswitch/ovs/blob/39fbd2c3f0392811689ec780f09baf90faceb877/lib/netdev-linux.c#L1238
> >
> > This fixes your problem for -Wflexible-array-array but the same
> > will happen with -Wflexible-array-sizeof (and you're using sizeof()
> > on such flexible unions) and -Wflexible-array-nested.
>
> I thought that it will fail some other checks too, but for some reason
> it doesn't. But, yes, you're right, It sounds safer to disable all
> of them to avoid possible issues in the future since we're actually
> using these unions.
Thanks for giving it a try.
> > options.c | 2 ++
> > options.h | 1 +
> > sparse.1 | 9 +++++++++
> > symbol.c | 2 +-
> > validation/flex-union-array-no.c | 9 +++++++++
> > validation/flex-union-array-yes.c | 11 +++++++++++
> > validation/flex-union-array.h | 11 +++++++++++
> > 7 files changed, 44 insertions(+), 1 deletion(-)
> > create mode 100644 validation/flex-union-array-no.c
> > create mode 100644 validation/flex-union-array-yes.c
> > create mode 100644 validation/flex-union-array.h
>
> Since you renamed the option, it might make sense to rename
> files to 'flex-array-union...'.
Well yes, I left them more or less on purpose but renamed them now.
-- Luc
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
