Re: [PATCH net-next v6 4/5] ip_tunnel: Preserve pointer const in ip_tunnel_info_opts

From: Eric Dumazet
Date: Thu Mar 09 2023 - 10:08:07 EST


On Thu, Mar 9, 2023 at 3:59 PM Eric Dumazet <edumazet@xxxxxxxxxx> wrote:
>
> On Thu, Mar 9, 2023 at 2:48 PM Gavin Li <gavinl@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> > 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.
>
> 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 :

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)