Messages in this thread | | | Date | Fri, 10 Mar 2023 12:53:46 +0100 | From | Simon Horman <> | Subject | Re: [PATCH net-next v6 4/5] ip_tunnel: Preserve pointer const in ip_tunnel_info_opts |
| |
On Thu, Mar 09, 2023 at 04:05:13PM +0100, Eric Dumazet wrote: > On Thu, Mar 9, 2023 at 3:59 PM Eric Dumazet <edumazet@google.com> wrote: > > > > On Thu, Mar 9, 2023 at 2:48 PM Gavin Li <gavinl@nvidia.com> wrote: > > > > > > Change ip_tunnel_info_opts( ) from static function to macro to cast return > > > value and preserve the const-ness of the pointer. > > > > > > Signed-off-by: Gavin Li <gavinl@nvidia.com> > > > --- > > > include/net/ip_tunnels.h | 11 ++++++----- > > > 1 file changed, 6 insertions(+), 5 deletions(-) > > > > > > diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h > > > index fca357679816..3e5c102b841f 100644 > > > --- a/include/net/ip_tunnels.h > > > +++ b/include/net/ip_tunnels.h > > > @@ -67,6 +67,12 @@ struct ip_tunnel_key { > > > GENMASK((sizeof_field(struct ip_tunnel_info, \ > > > options_len) * BITS_PER_BYTE) - 1, 0) > > > > > > +#define ip_tunnel_info_opts(info) \ > > > + _Generic(info, \ > > > + const typeof(*(info)) * : ((const void *)((info) + 1)),\ > > > + default : ((void *)((info) + 1)) \ > > > + ) > > > + > > > > Hmm... > > > > This looks quite dangerous, we lost type safety with the 'default' > > case, with all these casts.
Thanks Eric,
FWIIW, the feedback I gave earlier, which partly brought about this change, was based around avoiding casts...
> > > > What about using something cleaner instead ? > > > > (Not sure why we do not have an available generic helper for this kind > > of repetitive things) > > > > Or more exactly :
... So, yes, this looks better to me.
> diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h > index fca3576798166416982ee6a9100b003810c49830..17fc6c8f7e0b9e5303c1fb9e5dad77c5310e01a9 > 100644 > --- a/include/net/ip_tunnels.h > +++ b/include/net/ip_tunnels.h > @@ -485,10 +485,11 @@ static inline void iptunnel_xmit_stats(struct > net_device *dev, int pkt_len) > } > } > > -static inline void *ip_tunnel_info_opts(struct ip_tunnel_info *info) > -{ > - return info + 1; > -} > +#define ip_tunnel_info_opts(info) \ > + (_Generic(info, \ > + const struct ip_tunnel_info * : (const void *)((info) > + 1), \ > + struct ip_tunnel_info * : (void *)((info) + 1)) \ > + ) > > static inline void ip_tunnel_info_opts_get(void *to, > const struct ip_tunnel_info *info) >
| |