[ovs-dev] [PATCH] ovsdb-idlc: Use ALIGNED_CAST to avoid spurious warnings for index rows.

[Ben Pfaff]

On Thu, Sep 13, 2018 at 04:26:52PM -0700, Han Zhou wrote:
> On Thu, Sep 13, 2018 at 2:54 PM Ben Pfaff <blp at ovn.org> wrote:
> >
> > On Wed, Sep 12, 2018 at 03:47:05PM -0700, Han Zhou wrote:
> > > On Wed, Sep 12, 2018 at 2:18 PM Ben Pfaff <blp at ovn.org> wrote:
> > > >
> > > > On Tue, Sep 11, 2018 at 11:32:25PM -0700, Han Zhou wrote:
> > > > > On Mon, Sep 10, 2018 at 1:01 PM Ben Pfaff <blp at ovn.org> wrote:
> > > > > >
> > > > > > The *_index_init_row() function casts a generic ovsdb_idl_row
> pointer
> > > to
> > > > > > a specific type of row pointer.  This can cause an increase in
> > > required
> > > > > > alignment with some kinds of data on some architectures.  GCC
> > > complains,
> > > > > > e.g.:
> > > > > >
> > > > > >     lib/vswitch-idl.c: In function
> > > > > 'ovsrec_flow_sample_collector_set_index_init_row'
> > > > > >     lib/vswitch-idl.c:9277:12: warning: cast increases required
> > > alignment
> > > > > of target
> > > > >
> > > > > Hi Ben, could you share on which compiler/version you see this
> warning?
> > > >
> > > > It's on non-x86 architectures, e.g. mipsel.  I don't think it's
> > > > particularly sensitive to GCC version but this is with version 8.2.0:
> > > >
> > >
> https://buildd.debian.org/status/fetch.php?pkg=openvswitch&arch=mipsel&ver=2.10.0%2B2018.08.28%2Bgit.8ca7c82b7d%2Bds1-3&stamp=1536673373&raw=0
> > >
> > > I see, thanks. But I wonder why it complains only for this particular
> type
> > > cast - there are many type casts in other places.
> >
> > Without actually looking carefully, I think it's because struct
> > ovsdb_idl_row doesn't have any 64-bit members but the row type in
> > question does ("int64_t id;").  That makes it look to the compiler like
> > we're doing something questionable.
> >
> > > Do you have a hint on when should the ALIGNED_CAST be used, or do we
> > > have to rely on the compiler warnings - and it is not easy to get
> > > since most developers doesn't compile for other archs.
> >
> > The sole value of ALIGNED_CAST is suppressing compiler warnings, in the
> > case where the compiler's warning is uninformed.  Thus, there's no good
> > reason for developers to add these proactively anyway.
> 
> So it seems only in rare situation will this happen, and it is not a too
> bad idea to rely on upstream CI system to find out.
> 
> BTW, the questions are only about how to avoid this in the future when we
> submitting code. For the patch itself:
> 
> Acked-by: hzhou8 at ebay.com

Thanks, I applied this to master and branch-2.10, and sent out a patch
to better document ALIGNED_CAST.