On Sun, Oct 13, 2019 at 12:58:04PM -0700, Eric Dumazet wrote:
>
>
> On 10/12/19 5:30 PM, Vito Caputo wrote:
> > Return directly from all switch cases, no point in storing in err.
> >
> > Signed-off-by: Vito Caputo <vcap...@pengaru.com>
> > ---
> > net/core/skbuff.c | 15 +++------------
> > 1 file changed, 3 insertions(+), 12 deletions(-)
> >
> > diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> > index f5f904f46893..c59b68a413b5 100644
> > --- a/net/core/skbuff.c
> > +++ b/net/core/skbuff.c
> > @@ -4888,23 +4888,14 @@ static int skb_checksum_setup_ipv6(struct sk_buff
> > *skb, bool recalculate)
> > */
> > int skb_checksum_setup(struct sk_buff *skb, bool recalculate)
> > {
> > - int err;
> > -
> > switch (skb->protocol) {
> > case htons(ETH_P_IP):
> > - err = skb_checksum_setup_ipv4(skb, recalculate);
> > - break;
> > -
> > + return skb_checksum_setup_ipv4(skb, recalculate);
> > case htons(ETH_P_IPV6):
> > - err = skb_checksum_setup_ipv6(skb, recalculate);
> > - break;
> > -
> > + return skb_checksum_setup_ipv6(skb, recalculate);
> > default:
> > - err = -EPROTO;
> > - break;
> > + return -EPROTO;
> > }
> > -
> > - return err;
> > }
> > EXPORT_SYMBOL(skb_checksum_setup);
>
>
> We prefer having a single return point in a function, if possible.
>
> The err variable would make easier for debugging support,
> if say a developer needs to trace this function.
>
Except there are examples under net/core of precisely this pattern, e.g.:
---
__be32 flow_get_u32_src(const struct flow_keys *flow)
{
switch (flow->control.addr_type) {
case FLOW_DISSECTOR_KEY_IPV4_ADDRS:
return flow->addrs.v4addrs.src;
case FLOW_DISSECTOR_KEY_IPV6_ADDRS:
return (__force __be32)ipv6_addr_hash(
&flow->addrs.v6addrs.src);
case FLOW_DISSECTOR_KEY_TIPC:
return flow->addrs.tipckey.key;
default:
return 0;
}
}
EXPORT_SYMBOL(flow_get_u32_src);
__be32 flow_get_u32_dst(const struct flow_keys *flow)
{
switch (flow->control.addr_type) {
case FLOW_DISSECTOR_KEY_IPV4_ADDRS:
return flow->addrs.v4addrs.dst;
case FLOW_DISSECTOR_KEY_IPV6_ADDRS:
return (__force __be32)ipv6_addr_hash(
&flow->addrs.v6addrs.dst);
default:
return 0;
}
}
EXPORT_SYMBOL(flow_get_u32_dst);
---
This compact form of mapping is found throughout the kernel, is
skb_checksum_setup() special?
Regards,
Vito Caputo
