On Thu, Sep 13, 2018 at 04:26:52PM -0700, Han Zhou wrote: > On Thu, Sep 13, 2018 at 2:54 PM Ben Pfaff <[email protected]> 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 <[email protected]> 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 <[email protected]> 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: [email protected]
Thanks, I applied this to master and branch-2.10, and sent out a patch to better document ALIGNED_CAST. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
