On Thu, 21 Oct 2021 19:16:19 +0300
Eli Britstein <el...@nvidia.com> wrote:
> On 10/21/2021 6:48 PM, Stephen Hemminger wrote:
> > External email: Use caution opening links or attachments
> >
> >
> > On Thu, 21 Oct 2021 11:51:30 +0300
> > Eli Britstein <el...@nvidia.com> wrote:
> >
> >> In rte_vlan_insert there is a casting of rte_pktmbuf_prepend returned
> >> value to (struct rte_ether_hdr *), which causes cast-align warning when
> >> using strict cast align flag with supporting gcc:
> >> gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0
> >> CFLAGS="-Wcast-align=strict" make V=1 -C examples/l2fwd clean static
> >>
> >> In file included from main.c:35:
> >> /dpdk/build/include/rte_ether.h:370:7: warning: cast increases required
> >> alignment of target type [-Wcast-align]
> >> 370 | nh = (struct rte_ether_hdr *)
> >> | ^
> >>
> >> As the code assumes correct alignment, add first a (void *) casting, to
> >> avoid the warning.
> >>
> >> Fixes: c974021a5949 ("ether: add soft vlan encap/decap")
> >> Cc: sta...@dpdk.org
> >>
> >> Signed-off-by: Eli Britstein <el...@nvidia.com>
> >> Acked-by: Olivier Matz <olivier.m...@6wind.com>
> > After cast to void * the second cast is not necessary.
> >
> > nh = (void *)rte_pktmbuf_prepend(...)
> >
> > Ideally rte_pktmbuf_prepend() should return void * but that is
> > an API change.
>
> Removing the second cast, it is silently done anyway, as 'nh' is of type
> 'struct rte_ether_hdr *'.
>
> Going with this approach (I can also do it for patch 3/3), we can change
> rte_pktmbuf_prepend to return (void *), and let the applications using
> it do the silent cast.
>
> What do you think?
Changing return type is an API change so it would need the whole
multistep process.
I overstated a little, it turns out the cast is necessary when header
is included by C++ code. C++ is pickier and doesn't allow void * to be
converted to other type by assignment.
Probably best to stick with what you originally proposed.
Gcc does have a bunch of alignment attribute types that could also fix this
but that gets even messier.
