Fri, Sep 15, 2023 at 10:12:47AM CEST, dan.carpenter@xxxxxxxxxx wrote:
>Hello Jiri Pirko,
>
>The patch df38dafd2559: "devlink: implement shared buffer occupancy
>monitoring interface" from Apr 14, 2016 (linux-next), leads to the
>following Smatch static checker warning:
>
> net/devlink/sb.c:699 devlink_nl_sb_tc_pool_bind_fill()
> warn: missing unwind goto?
>
>net/devlink/sb.c
> 651 static int
> 652 devlink_nl_sb_tc_pool_bind_fill(struct sk_buff *msg, struct devlink *devlink,
> 653 struct devlink_port *devlink_port,
> 654 struct devlink_sb *devlink_sb, u16 tc_index,
> 655 enum devlink_sb_pool_type pool_type,
> 656 enum devlink_command cmd,
> 657 u32 portid, u32 seq, int flags)
> 658 {
> 659 const struct devlink_ops *ops = devlink->ops;
> 660 u16 pool_index;
> 661 u32 threshold;
> 662 void *hdr;
> 663 int err;
> 664
> 665 err = ops->sb_tc_pool_bind_get(devlink_port, devlink_sb->index,
> 666 tc_index, pool_type,
> 667 &pool_index, &threshold);
> 668 if (err)
> 669 return err;
> 670
> 671 hdr = genlmsg_put(msg, portid, seq, &devlink_nl_family, flags, cmd);
> 672 if (!hdr)
> 673 return -EMSGSIZE;
> 674
> 675 if (devlink_nl_put_handle(msg, devlink))
> 676 goto nla_put_failure;
> 677 if (nla_put_u32(msg, DEVLINK_ATTR_PORT_INDEX, devlink_port->index))
> 678 goto nla_put_failure;
> 679 if (nla_put_u32(msg, DEVLINK_ATTR_SB_INDEX, devlink_sb->index))
> 680 goto nla_put_failure;
> 681 if (nla_put_u16(msg, DEVLINK_ATTR_SB_TC_INDEX, tc_index))
> 682 goto nla_put_failure;
> 683 if (nla_put_u8(msg, DEVLINK_ATTR_SB_POOL_TYPE, pool_type))
> 684 goto nla_put_failure;
> 685 if (nla_put_u16(msg, DEVLINK_ATTR_SB_POOL_INDEX, pool_index))
> 686 goto nla_put_failure;
> 687 if (nla_put_u32(msg, DEVLINK_ATTR_SB_THRESHOLD, threshold))
> 688 goto nla_put_failure;
> 689
> 690 if (ops->sb_occ_tc_port_bind_get) {
> 691 u32 cur;
> 692 u32 max;
> 693
> 694 err = ops->sb_occ_tc_port_bind_get(devlink_port,
> 695 devlink_sb->index,
> 696 tc_index, pool_type,
> 697 &cur, &max);
> 698 if (err && err != -EOPNOTSUPP)
>--> 699 return err;
>
>Moving code around means re-visiting all the static checker warnings.
>:) Should this return genlmsg_cancel(msg, hdr) before returning?
Well, as the message is freed when this function returns !=0,
genlmsg_cancel() is optional if not pointless here.
>
> 700 if (!err) {
> 701 if (nla_put_u32(msg, DEVLINK_ATTR_SB_OCC_CUR, cur))
> 702 goto nla_put_failure;
> 703 if (nla_put_u32(msg, DEVLINK_ATTR_SB_OCC_MAX, max))
> 704 goto nla_put_failure;
> 705 }
> 706 }
> 707
> 708 genlmsg_end(msg, hdr);
> 709 return 0;
> 710
> 711 nla_put_failure:
> 712 genlmsg_cancel(msg, hdr);
> 713 return -EMSGSIZE;
> 714 }
>
>regards,
>dan carpenter
