* [RFC PATCH 1/2] net: netlink: add nla_get_*_default() accessors
@ 2024-10-24 11:18 Johannes Berg
2024-10-24 11:18 ` [RFC PATCH 2/2] net: convert to nla_get_*_default() Johannes Berg
` (5 more replies)
0 siblings, 6 replies; 17+ messages in thread
From: Johannes Berg @ 2024-10-24 11:18 UTC (permalink / raw)
To: linux-wireless, netdev; +Cc: Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
There are quite a number of places that use patterns
such as
if (attr)
val = nla_get_u16(attr);
else
val = DEFAULT;
Add nla_get_u16_default() and friends like that to
not have to type this out all the time.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
include/net/netlink.h | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/include/net/netlink.h b/include/net/netlink.h
index db6af207287c..b15bd0437945 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -142,6 +142,8 @@
* nla_get_flag(nla) return 1 if flag is true
* nla_get_msecs(nla) get payload for a msecs attribute
*
+ * The same functions also exist with _default().
+ *
* Attribute Misc:
* nla_memcpy(dest, nla, count) copy attribute into memory
* nla_memcmp(nla, data, size) compare attribute with memory area
@@ -1867,6 +1869,31 @@ static inline unsigned long nla_get_msecs(const struct nlattr *nla)
return msecs_to_jiffies((unsigned long) msecs);
}
+#define MAKE_NLA_GET_DEFAULT(tp, fn) \
+static inline tp fn##_default(const struct nlattr *nla, \
+ tp defvalue) \
+{ \
+ if (!nla) \
+ return defvalue; \
+ return n(nla); \
+}
+
+MAKE_NLA_GET_DEFAULT(u8, nla_get_u8);
+MAKE_NLA_GET_DEFAULT(u16, nla_get_u16);
+MAKE_NLA_GET_DEFAULT(u32, nla_get_u32);
+MAKE_NLA_GET_DEFAULT(u64, nla_get_u64);
+MAKE_NLA_GET_DEFAULT(unsigned long, nla_get_msecs);
+MAKE_NLA_GET_DEFAULT(s8, nla_get_s8);
+MAKE_NLA_GET_DEFAULT(s16, nla_get_s16);
+MAKE_NLA_GET_DEFAULT(s32, nla_get_s32);
+MAKE_NLA_GET_DEFAULT(s64, nla_get_s64);
+MAKE_NLA_GET_DEFAULT(s16, nla_get_le16);
+MAKE_NLA_GET_DEFAULT(s32, nla_get_le32);
+MAKE_NLA_GET_DEFAULT(s64, nla_get_le64);
+MAKE_NLA_GET_DEFAULT(s16, nla_get_be16);
+MAKE_NLA_GET_DEFAULT(s32, nla_get_be32);
+MAKE_NLA_GET_DEFAULT(s64, nla_get_be64);
+
/**
* nla_get_in_addr - return payload of IPv4 address attribute
* @nla: IPv4 address netlink attribute
--
2.47.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [RFC PATCH 2/2] net: convert to nla_get_*_default()
2024-10-24 11:18 [RFC PATCH 1/2] net: netlink: add nla_get_*_default() accessors Johannes Berg
@ 2024-10-24 11:18 ` Johannes Berg
2024-10-24 12:11 ` Johannes Berg
` (2 more replies)
2024-10-24 11:29 ` [RFC PATCH 1/2] net: netlink: add nla_get_*_default() accessors Johannes Berg
` (4 subsequent siblings)
5 siblings, 3 replies; 17+ messages in thread
From: Johannes Berg @ 2024-10-24 11:18 UTC (permalink / raw)
To: linux-wireless, netdev; +Cc: Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
This is mostly to illustrate, done with the following spatch:
@@
expression attr, def;
expression val;
identifier fn =~ "^nla_get_.*";
fresh identifier dfn = fn ## "_default";
@@
(
-if (attr)
- val = fn(attr);
-else
- val = def;
+val = dfn(attr, def);
|
-if (!attr)
- val = def;
-else
- val = fn(attr);
+val = dfn(attr, def);
)
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
net/8021q/vlan_netlink.c | 6 ++--
net/core/fib_rules.c | 12 +++----
net/core/rtnetlink.c | 5 +--
net/devlink/dev.c | 6 ++--
net/hsr/hsr_netlink.c | 5 +--
net/ieee802154/nl-mac.c | 15 ++------
net/ieee802154/nl802154.c | 24 +++++--------
net/ipv4/nexthop.c | 10 ++----
net/ipv6/addrconf.c | 5 +--
net/ipv6/ila/ila_xlat.c | 13 +++----
net/ipv6/ioam6.c | 12 +++----
net/ipv6/ioam6_iptunnel.c | 6 ++--
net/netfilter/ipvs/ip_vs_ctl.c | 5 +--
net/netfilter/nf_nat_core.c | 6 ++--
net/netfilter/nft_tunnel.c | 5 +--
net/netlabel/netlabel_mgmt.c | 12 ++-----
net/nfc/netlink.c | 6 ++--
net/sched/act_gate.c | 11 ++----
net/sched/sch_gred.c | 7 ++--
net/sched/sch_htb.c | 6 ++--
net/sched/sch_netem.c | 6 ++--
net/sched/sch_qfq.c | 5 +--
net/wireless/nl80211.c | 65 ++++++++++------------------------
net/xfrm/xfrm_user.c | 6 ++--
24 files changed, 74 insertions(+), 185 deletions(-)
diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c
index cf5219df7903..134419667d59 100644
--- a/net/8021q/vlan_netlink.c
+++ b/net/8021q/vlan_netlink.c
@@ -161,10 +161,8 @@ static int vlan_newlink(struct net *src_net, struct net_device *dev,
return -ENODEV;
}
- if (data[IFLA_VLAN_PROTOCOL])
- proto = nla_get_be16(data[IFLA_VLAN_PROTOCOL]);
- else
- proto = htons(ETH_P_8021Q);
+ proto = nla_get_be16_default(data[IFLA_VLAN_PROTOCOL],
+ htons(ETH_P_8021Q));
vlan->vlan_proto = proto;
vlan->vlan_id = nla_get_u16(data[IFLA_VLAN_ID]);
diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
index 154a2681f55c..9831ccc02d5a 100644
--- a/net/core/fib_rules.c
+++ b/net/core/fib_rules.c
@@ -601,15 +601,11 @@ static int fib_nl2rule(struct sk_buff *skb, struct nlmsghdr *nlh,
nlrule->action = frh->action;
nlrule->flags = frh->flags;
nlrule->table = frh_get_table(frh, tb);
- if (tb[FRA_SUPPRESS_PREFIXLEN])
- nlrule->suppress_prefixlen = nla_get_u32(tb[FRA_SUPPRESS_PREFIXLEN]);
- else
- nlrule->suppress_prefixlen = -1;
+ nlrule->suppress_prefixlen = nla_get_u32_default(tb[FRA_SUPPRESS_PREFIXLEN],
+ -1);
- if (tb[FRA_SUPPRESS_IFGROUP])
- nlrule->suppress_ifgroup = nla_get_u32(tb[FRA_SUPPRESS_IFGROUP]);
- else
- nlrule->suppress_ifgroup = -1;
+ nlrule->suppress_ifgroup = nla_get_u32_default(tb[FRA_SUPPRESS_IFGROUP],
+ -1);
if (tb[FRA_GOTO]) {
if (nlrule->action != FR_ACT_GOTO) {
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 6d68247aea70..ddba16e1c849 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2871,10 +2871,7 @@ static int do_setlink(const struct sk_buff *skb,
goto errout;
}
- if (tb[IFLA_NEW_IFINDEX])
- new_ifindex = nla_get_s32(tb[IFLA_NEW_IFINDEX]);
- else
- new_ifindex = 0;
+ new_ifindex = nla_get_s32_default(tb[IFLA_NEW_IFINDEX], 0);
err = __dev_change_net_namespace(dev, net, pat, new_ifindex);
put_net(net);
diff --git a/net/devlink/dev.c b/net/devlink/dev.c
index 13c73f50da3d..23cfa163b9a9 100644
--- a/net/devlink/dev.c
+++ b/net/devlink/dev.c
@@ -531,10 +531,8 @@ int devlink_nl_reload_doit(struct sk_buff *skb, struct genl_info *info)
return err;
}
- if (info->attrs[DEVLINK_ATTR_RELOAD_ACTION])
- action = nla_get_u8(info->attrs[DEVLINK_ATTR_RELOAD_ACTION]);
- else
- action = DEVLINK_RELOAD_ACTION_DRIVER_REINIT;
+ action = nla_get_u8_default(info->attrs[DEVLINK_ATTR_RELOAD_ACTION],
+ DEVLINK_RELOAD_ACTION_DRIVER_REINIT);
if (!devlink_reload_action_is_supported(devlink, action)) {
NL_SET_ERR_MSG(info->extack, "Requested reload action is not supported by the driver");
diff --git a/net/hsr/hsr_netlink.c b/net/hsr/hsr_netlink.c
index f6ff0b61e08a..3d4630e876e1 100644
--- a/net/hsr/hsr_netlink.c
+++ b/net/hsr/hsr_netlink.c
@@ -82,10 +82,7 @@ static int hsr_newlink(struct net *src_net, struct net_device *dev,
return -EINVAL;
}
- if (!data[IFLA_HSR_MULTICAST_SPEC])
- multicast_spec = 0;
- else
- multicast_spec = nla_get_u8(data[IFLA_HSR_MULTICAST_SPEC]);
+ multicast_spec = nla_get_u8_default(data[IFLA_HSR_MULTICAST_SPEC], 0);
if (data[IFLA_HSR_PROTOCOL])
proto = nla_get_u8(data[IFLA_HSR_PROTOCOL]);
diff --git a/net/ieee802154/nl-mac.c b/net/ieee802154/nl-mac.c
index 29bf97640166..74ef0a310afb 100644
--- a/net/ieee802154/nl-mac.c
+++ b/net/ieee802154/nl-mac.c
@@ -202,10 +202,7 @@ int ieee802154_associate_req(struct sk_buff *skb, struct genl_info *info)
addr.pan_id = nla_get_shortaddr(
info->attrs[IEEE802154_ATTR_COORD_PAN_ID]);
- if (info->attrs[IEEE802154_ATTR_PAGE])
- page = nla_get_u8(info->attrs[IEEE802154_ATTR_PAGE]);
- else
- page = 0;
+ page = nla_get_u8_default(info->attrs[IEEE802154_ATTR_PAGE], 0);
ret = ieee802154_mlme_ops(dev)->assoc_req(dev, &addr,
nla_get_u8(info->attrs[IEEE802154_ATTR_CHANNEL]),
@@ -338,10 +335,7 @@ int ieee802154_start_req(struct sk_buff *skb, struct genl_info *info)
blx = nla_get_u8(info->attrs[IEEE802154_ATTR_BAT_EXT]);
coord_realign = nla_get_u8(info->attrs[IEEE802154_ATTR_COORD_REALIGN]);
- if (info->attrs[IEEE802154_ATTR_PAGE])
- page = nla_get_u8(info->attrs[IEEE802154_ATTR_PAGE]);
- else
- page = 0;
+ page = nla_get_u8_default(info->attrs[IEEE802154_ATTR_PAGE], 0);
if (addr.short_addr == cpu_to_le16(IEEE802154_ADDR_BROADCAST)) {
ieee802154_nl_start_confirm(dev, IEEE802154_NO_SHORT_ADDRESS);
@@ -388,10 +382,7 @@ int ieee802154_scan_req(struct sk_buff *skb, struct genl_info *info)
channels = nla_get_u32(info->attrs[IEEE802154_ATTR_CHANNELS]);
duration = nla_get_u8(info->attrs[IEEE802154_ATTR_DURATION]);
- if (info->attrs[IEEE802154_ATTR_PAGE])
- page = nla_get_u8(info->attrs[IEEE802154_ATTR_PAGE]);
- else
- page = 0;
+ page = nla_get_u8_default(info->attrs[IEEE802154_ATTR_PAGE], 0);
ret = ieee802154_mlme_ops(dev)->scan_req(dev, type, channels,
page, duration);
diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index 7eb37de3add2..694356a1fc91 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c
@@ -1438,22 +1438,16 @@ static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
}
/* Use current page by default */
- if (info->attrs[NL802154_ATTR_PAGE])
- request->page = nla_get_u8(info->attrs[NL802154_ATTR_PAGE]);
- else
- request->page = wpan_phy->current_page;
+ request->page = nla_get_u8_default(info->attrs[NL802154_ATTR_PAGE],
+ wpan_phy->current_page);
/* Scan all supported channels by default */
- if (info->attrs[NL802154_ATTR_SCAN_CHANNELS])
- request->channels = nla_get_u32(info->attrs[NL802154_ATTR_SCAN_CHANNELS]);
- else
- request->channels = wpan_phy->supported.channels[request->page];
+ request->channels = nla_get_u32_default(info->attrs[NL802154_ATTR_SCAN_CHANNELS],
+ wpan_phy->supported.channels[request->page]);
/* Use maximum duration order by default */
- if (info->attrs[NL802154_ATTR_SCAN_DURATION])
- request->duration = nla_get_u8(info->attrs[NL802154_ATTR_SCAN_DURATION]);
- else
- request->duration = IEEE802154_MAX_SCAN_DURATION;
+ request->duration = nla_get_u8_default(info->attrs[NL802154_ATTR_SCAN_DURATION],
+ IEEE802154_MAX_SCAN_DURATION);
err = rdev_trigger_scan(rdev, request);
if (err) {
@@ -1598,10 +1592,8 @@ nl802154_send_beacons(struct sk_buff *skb, struct genl_info *info)
request->wpan_phy = wpan_phy;
/* Use maximum duration order by default */
- if (info->attrs[NL802154_ATTR_BEACON_INTERVAL])
- request->interval = nla_get_u8(info->attrs[NL802154_ATTR_BEACON_INTERVAL]);
- else
- request->interval = IEEE802154_MAX_SCAN_DURATION;
+ request->interval = nla_get_u8_default(info->attrs[NL802154_ATTR_BEACON_INTERVAL],
+ IEEE802154_MAX_SCAN_DURATION);
err = rdev_send_beacons(rdev, request);
if (err) {
diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
index 93aaea0006ba..3a520e158c47 100644
--- a/net/ipv4/nexthop.c
+++ b/net/ipv4/nexthop.c
@@ -3248,10 +3248,7 @@ static int nh_valid_get_del_req(const struct nlmsghdr *nlh,
}
if (op_flags) {
- if (tb[NHA_OP_FLAGS])
- *op_flags = nla_get_u32(tb[NHA_OP_FLAGS]);
- else
- *op_flags = 0;
+ *op_flags = nla_get_u32_default(tb[NHA_OP_FLAGS], 0);
}
return 0;
@@ -3433,10 +3430,7 @@ static int nh_valid_dump_req(const struct nlmsghdr *nlh,
if (err < 0)
return err;
- if (tb[NHA_OP_FLAGS])
- filter->op_flags = nla_get_u32(tb[NHA_OP_FLAGS]);
- else
- filter->op_flags = 0;
+ filter->op_flags = nla_get_u32_default(tb[NHA_OP_FLAGS], 0);
return __nh_valid_dump_req(nlh, tb, filter, cb->extack);
}
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 94dceac52884..bb921602d06c 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -5018,10 +5018,7 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh,
return -ENODEV;
}
- if (tb[IFA_FLAGS])
- cfg.ifa_flags = nla_get_u32(tb[IFA_FLAGS]);
- else
- cfg.ifa_flags = ifm->ifa_flags;
+ cfg.ifa_flags = nla_get_u32_default(tb[IFA_FLAGS], ifm->ifa_flags);
/* We ignore other flags so far. */
cfg.ifa_flags &= IFA_F_NODAD | IFA_F_HOMEADDRESS |
diff --git a/net/ipv6/ila/ila_xlat.c b/net/ipv6/ila/ila_xlat.c
index 534a4498e280..7646e401c630 100644
--- a/net/ipv6/ila/ila_xlat.c
+++ b/net/ipv6/ila/ila_xlat.c
@@ -105,16 +105,11 @@ static int parse_nl_config(struct genl_info *info,
xp->ip.locator_match.v64 = (__force __be64)nla_get_u64(
info->attrs[ILA_ATTR_LOCATOR_MATCH]);
- if (info->attrs[ILA_ATTR_CSUM_MODE])
- xp->ip.csum_mode = nla_get_u8(info->attrs[ILA_ATTR_CSUM_MODE]);
- else
- xp->ip.csum_mode = ILA_CSUM_NO_ACTION;
+ xp->ip.csum_mode = nla_get_u8_default(info->attrs[ILA_ATTR_CSUM_MODE],
+ ILA_CSUM_NO_ACTION);
- if (info->attrs[ILA_ATTR_IDENT_TYPE])
- xp->ip.ident_type = nla_get_u8(
- info->attrs[ILA_ATTR_IDENT_TYPE]);
- else
- xp->ip.ident_type = ILA_ATYPE_USE_FORMAT;
+ xp->ip.ident_type = nla_get_u8_default(info->attrs[ILA_ATTR_IDENT_TYPE],
+ ILA_ATYPE_USE_FORMAT);
if (info->attrs[ILA_ATTR_IFINDEX])
xp->ifindex = nla_get_s32(info->attrs[ILA_ATTR_IFINDEX]);
diff --git a/net/ipv6/ioam6.c b/net/ipv6/ioam6.c
index 08c929513065..a84d332f952f 100644
--- a/net/ipv6/ioam6.c
+++ b/net/ipv6/ioam6.c
@@ -135,15 +135,11 @@ static int ioam6_genl_addns(struct sk_buff *skb, struct genl_info *info)
ns->id = id;
- if (!info->attrs[IOAM6_ATTR_NS_DATA])
- data32 = IOAM6_U32_UNAVAILABLE;
- else
- data32 = nla_get_u32(info->attrs[IOAM6_ATTR_NS_DATA]);
+ data32 = nla_get_u32_default(info->attrs[IOAM6_ATTR_NS_DATA],
+ IOAM6_U32_UNAVAILABLE);
- if (!info->attrs[IOAM6_ATTR_NS_DATA_WIDE])
- data64 = IOAM6_U64_UNAVAILABLE;
- else
- data64 = nla_get_u64(info->attrs[IOAM6_ATTR_NS_DATA_WIDE]);
+ data64 = nla_get_u64_default(info->attrs[IOAM6_ATTR_NS_DATA_WIDE],
+ IOAM6_U64_UNAVAILABLE);
ns->data = cpu_to_be32(data32);
ns->data_wide = cpu_to_be64(data64);
diff --git a/net/ipv6/ioam6_iptunnel.c b/net/ipv6/ioam6_iptunnel.c
index beb6b4cfc551..9d8422e350f8 100644
--- a/net/ipv6/ioam6_iptunnel.c
+++ b/net/ipv6/ioam6_iptunnel.c
@@ -142,10 +142,8 @@ static int ioam6_build_state(struct net *net, struct nlattr *nla,
}
}
- if (!tb[IOAM6_IPTUNNEL_MODE])
- mode = IOAM6_IPTUNNEL_MODE_INLINE;
- else
- mode = nla_get_u8(tb[IOAM6_IPTUNNEL_MODE]);
+ mode = nla_get_u8_default(tb[IOAM6_IPTUNNEL_MODE],
+ IOAM6_IPTUNNEL_MODE_INLINE);
if (tb[IOAM6_IPTUNNEL_SRC] && mode == IOAM6_IPTUNNEL_MODE_INLINE) {
NL_SET_ERR_MSG(extack, "no tunnel src expected with this mode");
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index dc6ddc4abbe2..7d13110ce188 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -3662,10 +3662,7 @@ static int ip_vs_genl_parse_dest(struct ip_vs_dest_user_kern *udest,
nla_memcpy(&udest->addr, nla_addr, sizeof(udest->addr));
udest->port = nla_get_be16(nla_port);
- if (nla_addr_family)
- udest->af = nla_get_u16(nla_addr_family);
- else
- udest->af = 0;
+ udest->af = nla_get_u16_default(nla_addr_family, 0);
/* If a full entry was requested, check for the additional fields */
if (full_entry) {
diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
index 4085c436e306..aad84aabd7f1 100644
--- a/net/netfilter/nf_nat_core.c
+++ b/net/netfilter/nf_nat_core.c
@@ -1090,10 +1090,8 @@ static int nf_nat_ipv4_nlattr_to_range(struct nlattr *tb[],
range->flags |= NF_NAT_RANGE_MAP_IPS;
}
- if (tb[CTA_NAT_V4_MAXIP])
- range->max_addr.ip = nla_get_be32(tb[CTA_NAT_V4_MAXIP]);
- else
- range->max_addr.ip = range->min_addr.ip;
+ range->max_addr.ip = nla_get_be32_default(tb[CTA_NAT_V4_MAXIP],
+ range->min_addr.ip);
return 0;
}
diff --git a/net/netfilter/nft_tunnel.c b/net/netfilter/nft_tunnel.c
index 5c6ed68cc6e0..681301b46aa4 100644
--- a/net/netfilter/nft_tunnel.c
+++ b/net/netfilter/nft_tunnel.c
@@ -497,10 +497,7 @@ static int nft_tunnel_obj_init(const struct nft_ctx *ctx,
}
if (tb[NFTA_TUNNEL_KEY_TOS])
info.key.tos = nla_get_u8(tb[NFTA_TUNNEL_KEY_TOS]);
- if (tb[NFTA_TUNNEL_KEY_TTL])
- info.key.ttl = nla_get_u8(tb[NFTA_TUNNEL_KEY_TTL]);
- else
- info.key.ttl = U8_MAX;
+ info.key.ttl = nla_get_u8_default(tb[NFTA_TUNNEL_KEY_TTL], U8_MAX);
if (tb[NFTA_TUNNEL_KEY_OPTS]) {
err = nft_tunnel_obj_opts_init(ctx, tb[NFTA_TUNNEL_KEY_OPTS],
diff --git a/net/netlabel/netlabel_mgmt.c b/net/netlabel/netlabel_mgmt.c
index 689eaa2afbec..ce7fb797c3e0 100644
--- a/net/netlabel/netlabel_mgmt.c
+++ b/net/netlabel/netlabel_mgmt.c
@@ -107,11 +107,8 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
switch (entry->def.type) {
case NETLBL_NLTYPE_UNLABELED:
- if (info->attrs[NLBL_MGMT_A_FAMILY])
- entry->family =
- nla_get_u16(info->attrs[NLBL_MGMT_A_FAMILY]);
- else
- entry->family = AF_UNSPEC;
+ entry->family = nla_get_u16_default(info->attrs[NLBL_MGMT_A_FAMILY],
+ AF_UNSPEC);
break;
case NETLBL_NLTYPE_CIPSOV4:
if (!info->attrs[NLBL_MGMT_A_CV4DOI])
@@ -601,10 +598,7 @@ static int netlbl_mgmt_listdef(struct sk_buff *skb, struct genl_info *info)
struct netlbl_dom_map *entry;
u16 family;
- if (info->attrs[NLBL_MGMT_A_FAMILY])
- family = nla_get_u16(info->attrs[NLBL_MGMT_A_FAMILY]);
- else
- family = AF_INET;
+ family = nla_get_u16_default(info->attrs[NLBL_MGMT_A_FAMILY], AF_INET);
ans_skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
if (ans_skb == NULL)
diff --git a/net/nfc/netlink.c b/net/nfc/netlink.c
index dd2ce73a24fb..b5c7c67361f9 100644
--- a/net/nfc/netlink.c
+++ b/net/nfc/netlink.c
@@ -942,10 +942,8 @@ static int nfc_genl_dep_link_up(struct sk_buff *skb, struct genl_info *info)
return -EINVAL;
idx = nla_get_u32(info->attrs[NFC_ATTR_DEVICE_INDEX]);
- if (!info->attrs[NFC_ATTR_TARGET_INDEX])
- tgt_idx = NFC_TARGET_IDX_ANY;
- else
- tgt_idx = nla_get_u32(info->attrs[NFC_ATTR_TARGET_INDEX]);
+ tgt_idx = nla_get_u32_default(info->attrs[NFC_ATTR_TARGET_INDEX],
+ NFC_TARGET_IDX_ANY);
comm = nla_get_u8(info->attrs[NFC_ATTR_COMM_MODE]);
diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c
index 1dd74125398a..91c0ec729823 100644
--- a/net/sched/act_gate.c
+++ b/net/sched/act_gate.c
@@ -190,15 +190,10 @@ static int fill_gate_entry(struct nlattr **tb, struct tcfg_gate_entry *entry,
entry->interval = interval;
- if (tb[TCA_GATE_ENTRY_IPV])
- entry->ipv = nla_get_s32(tb[TCA_GATE_ENTRY_IPV]);
- else
- entry->ipv = -1;
+ entry->ipv = nla_get_s32_default(tb[TCA_GATE_ENTRY_IPV], -1);
- if (tb[TCA_GATE_ENTRY_MAX_OCTETS])
- entry->maxoctets = nla_get_s32(tb[TCA_GATE_ENTRY_MAX_OCTETS]);
- else
- entry->maxoctets = -1;
+ entry->maxoctets = nla_get_s32_default(tb[TCA_GATE_ENTRY_MAX_OCTETS],
+ -1);
return 0;
}
diff --git a/net/sched/sch_gred.c b/net/sched/sch_gred.c
index 79ba9dc70254..7a10bb159113 100644
--- a/net/sched/sch_gred.c
+++ b/net/sched/sch_gred.c
@@ -750,11 +750,8 @@ static int gred_init(struct Qdisc *sch, struct nlattr *opt,
return -EINVAL;
}
- if (tb[TCA_GRED_LIMIT])
- sch->limit = nla_get_u32(tb[TCA_GRED_LIMIT]);
- else
- sch->limit = qdisc_dev(sch)->tx_queue_len
- * psched_mtu(qdisc_dev(sch));
+ sch->limit = nla_get_u32_default(tb[TCA_GRED_LIMIT],
+ qdisc_dev(sch)->tx_queue_len * psched_mtu(qdisc_dev(sch)));
if (qdisc_dev(sch)->netdev_ops->ndo_setup_tc) {
table->opt = kzalloc(sizeof(*table->opt), GFP_KERNEL);
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index ff3de37874e4..c3fa365cdd0a 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -1103,10 +1103,8 @@ static int htb_init(struct Qdisc *sch, struct nlattr *opt,
if (err < 0)
return err;
- if (tb[TCA_HTB_DIRECT_QLEN])
- q->direct_qlen = nla_get_u32(tb[TCA_HTB_DIRECT_QLEN]);
- else
- q->direct_qlen = qdisc_dev(sch)->tx_queue_len;
+ q->direct_qlen = nla_get_u32_default(tb[TCA_HTB_DIRECT_QLEN],
+ qdisc_dev(sch)->tx_queue_len);
if ((q->rate2quantum = gopt->rate2quantum) < 1)
q->rate2quantum = 1;
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 39382ee1e331..05418a8af5ff 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -1059,10 +1059,8 @@ static int netem_change(struct Qdisc *sch, struct nlattr *opt,
/* capping jitter to the range acceptable by tabledist() */
q->jitter = min_t(s64, abs(q->jitter), INT_MAX);
- if (tb[TCA_NETEM_PRNG_SEED])
- q->prng.seed = nla_get_u64(tb[TCA_NETEM_PRNG_SEED]);
- else
- q->prng.seed = get_random_u64();
+ q->prng.seed = nla_get_u64_default(tb[TCA_NETEM_PRNG_SEED],
+ get_random_u64());
prandom_seed_state(&q->prng.prng_state, q->prng.seed);
unlock:
diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index d584c0c25899..6a07cdbdb9e1 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -421,10 +421,7 @@ static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
if (err < 0)
return err;
- if (tb[TCA_QFQ_WEIGHT])
- weight = nla_get_u32(tb[TCA_QFQ_WEIGHT]);
- else
- weight = 1;
+ weight = nla_get_u32_default(tb[TCA_QFQ_WEIGHT], 1);
if (tb[TCA_QFQ_LMAX]) {
lmax = nla_get_u32(tb[TCA_QFQ_LMAX]);
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 4a8c3b6d49d1..14c8d6764f9e 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -3414,11 +3414,8 @@ static int _nl80211_parse_chandef(struct cfg80211_registered_device *rdev,
if (attrs[NL80211_ATTR_CENTER_FREQ1]) {
chandef->center_freq1 =
nla_get_u32(attrs[NL80211_ATTR_CENTER_FREQ1]);
- if (attrs[NL80211_ATTR_CENTER_FREQ1_OFFSET])
- chandef->freq1_offset = nla_get_u32(
- attrs[NL80211_ATTR_CENTER_FREQ1_OFFSET]);
- else
- chandef->freq1_offset = 0;
+ chandef->freq1_offset = nla_get_u32_default(attrs[NL80211_ATTR_CENTER_FREQ1_OFFSET],
+ 0);
}
if (attrs[NL80211_ATTR_CENTER_FREQ2])
chandef->center_freq2 =
@@ -7378,17 +7375,11 @@ static int nl80211_set_station(struct sk_buff *skb, struct genl_info *info)
if (info->attrs[NL80211_ATTR_VLAN_ID])
params.vlan_id = nla_get_u16(info->attrs[NL80211_ATTR_VLAN_ID]);
- if (info->attrs[NL80211_ATTR_STA_LISTEN_INTERVAL])
- params.listen_interval =
- nla_get_u16(info->attrs[NL80211_ATTR_STA_LISTEN_INTERVAL]);
- else
- params.listen_interval = -1;
+ params.listen_interval = nla_get_u16_default(info->attrs[NL80211_ATTR_STA_LISTEN_INTERVAL],
+ -1);
- if (info->attrs[NL80211_ATTR_STA_SUPPORT_P2P_PS])
- params.support_p2p_ps =
- nla_get_u8(info->attrs[NL80211_ATTR_STA_SUPPORT_P2P_PS]);
- else
- params.support_p2p_ps = -1;
+ params.support_p2p_ps = nla_get_u8_default(info->attrs[NL80211_ATTR_STA_SUPPORT_P2P_PS],
+ -1);
if (!info->attrs[NL80211_ATTR_MAC])
return -EINVAL;
@@ -7578,10 +7569,8 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
dev->ieee80211_ptr->iftype == NL80211_IFTYPE_P2P_GO;
}
- if (info->attrs[NL80211_ATTR_PEER_AID])
- params.aid = nla_get_u16(info->attrs[NL80211_ATTR_PEER_AID]);
- else
- params.aid = nla_get_u16(info->attrs[NL80211_ATTR_STA_AID]);
+ params.aid = nla_get_u16_default(info->attrs[NL80211_ATTR_PEER_AID],
+ nla_get_u16(info->attrs[NL80211_ATTR_STA_AID]));
if (info->attrs[NL80211_ATTR_STA_CAPABILITY]) {
params.capability =
@@ -8265,11 +8254,8 @@ static int nl80211_req_set_reg(struct sk_buff *skb, struct genl_info *info)
if (unlikely(!rcu_access_pointer(cfg80211_regdomain)))
return -EINPROGRESS;
- if (info->attrs[NL80211_ATTR_USER_REG_HINT_TYPE])
- user_reg_hint_type =
- nla_get_u32(info->attrs[NL80211_ATTR_USER_REG_HINT_TYPE]);
- else
- user_reg_hint_type = NL80211_USER_REG_HINT_USER;
+ user_reg_hint_type = nla_get_u32_default(info->attrs[NL80211_ATTR_USER_REG_HINT_TYPE],
+ NL80211_USER_REG_HINT_USER);
switch (user_reg_hint_type) {
case NL80211_USER_REG_HINT_USER:
@@ -11087,11 +11073,8 @@ static int nl80211_crypto_settings(struct cfg80211_registered_device *rdev,
nla_len(info->attrs[NL80211_ATTR_SAE_PASSWORD]);
}
- if (info->attrs[NL80211_ATTR_SAE_PWE])
- settings->sae_pwe =
- nla_get_u8(info->attrs[NL80211_ATTR_SAE_PWE]);
- else
- settings->sae_pwe = NL80211_SAE_PWE_UNSPECIFIED;
+ settings->sae_pwe = nla_get_u8_default(info->attrs[NL80211_ATTR_SAE_PWE],
+ NL80211_SAE_PWE_UNSPECIFIED);
return 0;
}
@@ -12347,10 +12330,8 @@ static int nl80211_disconnect(struct sk_buff *skb, struct genl_info *info)
dev->ieee80211_ptr->conn_owner_nlportid != info->snd_portid)
return -EPERM;
- if (!info->attrs[NL80211_ATTR_REASON_CODE])
- reason = WLAN_REASON_DEAUTH_LEAVING;
- else
- reason = nla_get_u16(info->attrs[NL80211_ATTR_REASON_CODE]);
+ reason = nla_get_u16_default(info->attrs[NL80211_ATTR_REASON_CODE],
+ WLAN_REASON_DEAUTH_LEAVING);
if (reason == 0)
return -EINVAL;
@@ -13696,10 +13677,7 @@ static int nl80211_parse_wowlan_tcp(struct cfg80211_registered_device *rdev,
cfg->dst = nla_get_in_addr(tb[NL80211_WOWLAN_TCP_DST_IPV4]);
memcpy(cfg->dst_mac, nla_data(tb[NL80211_WOWLAN_TCP_DST_MAC]),
ETH_ALEN);
- if (tb[NL80211_WOWLAN_TCP_SRC_PORT])
- port = nla_get_u16(tb[NL80211_WOWLAN_TCP_SRC_PORT]);
- else
- port = 0;
+ port = nla_get_u16_default(tb[NL80211_WOWLAN_TCP_SRC_PORT], 0);
#ifdef CONFIG_INET
/* allocate a socket and port for it and use it */
err = __sock_create(wiphy_net(&rdev->wiphy), PF_INET, SOCK_STREAM,
@@ -13910,11 +13888,8 @@ static int nl80211_set_wowlan(struct sk_buff *skb, struct genl_info *info)
pat_len < wowlan->pattern_min_len)
goto error;
- if (!pat_tb[NL80211_PKTPAT_OFFSET])
- pkt_offset = 0;
- else
- pkt_offset = nla_get_u32(
- pat_tb[NL80211_PKTPAT_OFFSET]);
+ pkt_offset = nla_get_u32_default(pat_tb[NL80211_PKTPAT_OFFSET],
+ 0);
if (pkt_offset > wowlan->max_pkt_offset)
goto error;
new_triggers.patterns[i].pkt_offset = pkt_offset;
@@ -14158,10 +14133,8 @@ static int nl80211_parse_coalesce_rule(struct cfg80211_registered_device *rdev,
pat_len < coalesce->pattern_min_len)
return -EINVAL;
- if (!pat_tb[NL80211_PKTPAT_OFFSET])
- pkt_offset = 0;
- else
- pkt_offset = nla_get_u32(pat_tb[NL80211_PKTPAT_OFFSET]);
+ pkt_offset = nla_get_u32_default(pat_tb[NL80211_PKTPAT_OFFSET],
+ 0);
if (pkt_offset > coalesce->max_pkt_offset)
return -EINVAL;
new_rule->patterns[i].pkt_offset = pkt_offset;
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 2b10a45ff124..2fa354cdd89f 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -763,10 +763,8 @@ static void xfrm_smark_init(struct nlattr **attrs, struct xfrm_mark *m)
{
if (attrs[XFRMA_SET_MARK]) {
m->v = nla_get_u32(attrs[XFRMA_SET_MARK]);
- if (attrs[XFRMA_SET_MARK_MASK])
- m->m = nla_get_u32(attrs[XFRMA_SET_MARK_MASK]);
- else
- m->m = 0xffffffff;
+ m->m = nla_get_u32_default(attrs[XFRMA_SET_MARK_MASK],
+ 0xffffffff);
} else {
m->v = m->m = 0;
}
--
2.47.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [RFC PATCH 1/2] net: netlink: add nla_get_*_default() accessors
2024-10-24 11:18 [RFC PATCH 1/2] net: netlink: add nla_get_*_default() accessors Johannes Berg
2024-10-24 11:18 ` [RFC PATCH 2/2] net: convert to nla_get_*_default() Johannes Berg
@ 2024-10-24 11:29 ` Johannes Berg
2024-10-24 12:51 ` Toke Høiland-Jørgensen
` (3 subsequent siblings)
5 siblings, 0 replies; 17+ messages in thread
From: Johannes Berg @ 2024-10-24 11:29 UTC (permalink / raw)
To: linux-wireless, netdev
On Thu, 2024-10-24 at 13:18 +0200, Johannes Berg wrote:
>
> +MAKE_NLA_GET_DEFAULT(u8, nla_get_u8);
> +MAKE_NLA_GET_DEFAULT(u16, nla_get_u16);
> +MAKE_NLA_GET_DEFAULT(u32, nla_get_u32);
> +MAKE_NLA_GET_DEFAULT(u64, nla_get_u64);
> +MAKE_NLA_GET_DEFAULT(unsigned long, nla_get_msecs);
> +MAKE_NLA_GET_DEFAULT(s8, nla_get_s8);
> +MAKE_NLA_GET_DEFAULT(s16, nla_get_s16);
> +MAKE_NLA_GET_DEFAULT(s32, nla_get_s32);
> +MAKE_NLA_GET_DEFAULT(s64, nla_get_s64);
> +MAKE_NLA_GET_DEFAULT(s16, nla_get_le16);
> +MAKE_NLA_GET_DEFAULT(s32, nla_get_le32);
> +MAKE_NLA_GET_DEFAULT(s64, nla_get_le64);
> +MAKE_NLA_GET_DEFAULT(s16, nla_get_be16);
> +MAKE_NLA_GET_DEFAULT(s32, nla_get_be32);
> +MAKE_NLA_GET_DEFAULT(s64, nla_get_be64);
>
I obviously messed that up completely, but you get the point ...
johannes
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH 2/2] net: convert to nla_get_*_default()
2024-10-24 11:18 ` [RFC PATCH 2/2] net: convert to nla_get_*_default() Johannes Berg
@ 2024-10-24 12:11 ` Johannes Berg
2024-10-24 14:28 ` Sabrina Dubroca
2024-10-24 13:41 ` Johannes Berg
2024-10-29 15:57 ` Jakub Kicinski
2 siblings, 1 reply; 17+ messages in thread
From: Johannes Berg @ 2024-10-24 12:11 UTC (permalink / raw)
To: linux-wireless, netdev
On Thu, 2024-10-24 at 13:18 +0200, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> This is mostly to illustrate, done with the following spatch:
>
And we can extend that and get bunch more:
@@
expression attr, def;
expression val;
identifier fn =~ "^nla_get_.*";
fresh identifier dfn = fn ## "_default";
@@
(
-if (attr)
- val = fn(attr);
-else
- val = def;
+val = dfn(attr, def);
|
-if (!attr)
- val = def;
-else
- val = fn(attr);
+val = dfn(attr, def);
|
-val = def;
... where != val;
-if (attr)
- val = fn(attr);
+val = dfn(attr, def);
|
-if (!attr)
- return def;
-return fn(attr);
+return dfn(attr, def);
)
(also just running it multiple times finds more instances?!)
johannes
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH 1/2] net: netlink: add nla_get_*_default() accessors
2024-10-24 11:18 [RFC PATCH 1/2] net: netlink: add nla_get_*_default() accessors Johannes Berg
2024-10-24 11:18 ` [RFC PATCH 2/2] net: convert to nla_get_*_default() Johannes Berg
2024-10-24 11:29 ` [RFC PATCH 1/2] net: netlink: add nla_get_*_default() accessors Johannes Berg
@ 2024-10-24 12:51 ` Toke Høiland-Jørgensen
2024-10-24 15:42 ` kernel test robot
` (2 subsequent siblings)
5 siblings, 0 replies; 17+ messages in thread
From: Toke Høiland-Jørgensen @ 2024-10-24 12:51 UTC (permalink / raw)
To: Johannes Berg, linux-wireless, netdev; +Cc: Johannes Berg
Johannes Berg <johannes@sipsolutions.net> writes:
> From: Johannes Berg <johannes.berg@intel.com>
>
> There are quite a number of places that use patterns
> such as
>
> if (attr)
> val = nla_get_u16(attr);
> else
> val = DEFAULT;
>
> Add nla_get_u16_default() and friends like that to
> not have to type this out all the time.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
I think this is an excellent idea! So typos/copy-paste errors aside:
Acked-by: Toke Høiland-Jørgensen <toke@kernel.org>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH 2/2] net: convert to nla_get_*_default()
2024-10-24 11:18 ` [RFC PATCH 2/2] net: convert to nla_get_*_default() Johannes Berg
2024-10-24 12:11 ` Johannes Berg
@ 2024-10-24 13:41 ` Johannes Berg
2024-10-24 14:48 ` Sabrina Dubroca
2024-10-29 15:57 ` Jakub Kicinski
2 siblings, 1 reply; 17+ messages in thread
From: Johannes Berg @ 2024-10-24 13:41 UTC (permalink / raw)
To: linux-wireless, netdev
Also just realized that some of the conversions are wrong, e.g.
> @@ -7378,17 +7375,11 @@ static int nl80211_set_station(struct sk_buff *skb, struct genl_info *info)
> if (info->attrs[NL80211_ATTR_VLAN_ID])
> params.vlan_id = nla_get_u16(info->attrs[NL80211_ATTR_VLAN_ID]);
>
> - if (info->attrs[NL80211_ATTR_STA_LISTEN_INTERVAL])
> - params.listen_interval =
> - nla_get_u16(info->attrs[NL80211_ATTR_STA_LISTEN_INTERVAL]);
> - else
> - params.listen_interval = -1;
> + params.listen_interval = nla_get_u16_default(info->attrs[NL80211_ATTR_STA_LISTEN_INTERVAL],
> + -1);
this one clearly is.
So need to be more careful with that :)
But at least the spatch gives some candidates.
johannes
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH 2/2] net: convert to nla_get_*_default()
2024-10-24 12:11 ` Johannes Berg
@ 2024-10-24 14:28 ` Sabrina Dubroca
2024-10-24 14:31 ` Johannes Berg
0 siblings, 1 reply; 17+ messages in thread
From: Sabrina Dubroca @ 2024-10-24 14:28 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, netdev
2024-10-24, 14:11:05 +0200, Johannes Berg wrote:
> On Thu, 2024-10-24 at 13:18 +0200, Johannes Berg wrote:
> > From: Johannes Berg <johannes.berg@intel.com>
> >
> > This is mostly to illustrate, done with the following spatch:
> >
>
> And we can extend that and get bunch more:
>
> @@
> expression attr, def;
> expression val;
> identifier fn =~ "^nla_get_.*";
> fresh identifier dfn = fn ## "_default";
> @@
> (
> -if (attr)
> - val = fn(attr);
> -else
> - val = def;
> +val = dfn(attr, def);
> |
> -if (!attr)
> - val = def;
> -else
> - val = fn(attr);
> +val = dfn(attr, def);
> |
> -val = def;
> ... where != val;
> -if (attr)
> - val = fn(attr);
> +val = dfn(attr, def);
> |
> -if (!attr)
> - return def;
> -return fn(attr);
> +return dfn(attr, def);
> )
Not really familiar with spatch, but I'm guessing this won't cover:
val = attr ? getter(attr) : default;
See macsec_validate_attr in drivers/net/macsec.c for some
examples. There are also some cases where we have "if (data &&
data[IFLA_MACSEC_*])" guarding the attribute fetch
(drivers/net/macvlan.c does that too), but I guess you can't really
cover that without adding some kind of "default_with_cond" helpers.
--
Sabrina
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH 2/2] net: convert to nla_get_*_default()
2024-10-24 14:28 ` Sabrina Dubroca
@ 2024-10-24 14:31 ` Johannes Berg
2024-10-24 14:40 ` Johannes Berg
0 siblings, 1 reply; 17+ messages in thread
From: Johannes Berg @ 2024-10-24 14:31 UTC (permalink / raw)
To: Sabrina Dubroca; +Cc: linux-wireless, netdev
On Thu, 2024-10-24 at 16:28 +0200, Sabrina Dubroca wrote:
>
>
> Not really familiar with spatch, but I'm guessing this won't cover:
> val = attr ? getter(attr) : default;
True, we could add
-val = attr ? fn(attr) : def;
+val = dfn(attr);
in the spatch for that.
But also see my other mail - the spatch can only be used to suggest
places to change, need to review them still due to integer type (both
signedness and width) issues.
> See macsec_validate_attr in drivers/net/macsec.c for some
> examples. There are also some cases where we have "if (data &&
> data[IFLA_MACSEC_*])" guarding the attribute fetch
> (drivers/net/macvlan.c does that too), but I guess you can't really
> cover that without adding some kind of "default_with_cond" helpers.
>
Yeah if there's some additional condition then I guess we just keep the
existing code.
johannes
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH 2/2] net: convert to nla_get_*_default()
2024-10-24 14:31 ` Johannes Berg
@ 2024-10-24 14:40 ` Johannes Berg
0 siblings, 0 replies; 17+ messages in thread
From: Johannes Berg @ 2024-10-24 14:40 UTC (permalink / raw)
To: Sabrina Dubroca; +Cc: linux-wireless, netdev
On Thu, 2024-10-24 at 16:31 +0200, Johannes Berg wrote:
> On Thu, 2024-10-24 at 16:28 +0200, Sabrina Dubroca wrote:
> >
> >
> > Not really familiar with spatch, but I'm guessing this won't cover:
> > val = attr ? getter(attr) : default;
>
> True, we could add
>
> -val = attr ? fn(attr) : def;
> +val = dfn(attr);
>
There are about 50 of those :) thanks!
johannes
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH 2/2] net: convert to nla_get_*_default()
2024-10-24 13:41 ` Johannes Berg
@ 2024-10-24 14:48 ` Sabrina Dubroca
2024-10-24 14:52 ` Johannes Berg
0 siblings, 1 reply; 17+ messages in thread
From: Sabrina Dubroca @ 2024-10-24 14:48 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, netdev
2024-10-24, 15:41:59 +0200, Johannes Berg wrote:
>
> Also just realized that some of the conversions are wrong, e.g.
>
> > @@ -7378,17 +7375,11 @@ static int nl80211_set_station(struct sk_buff *skb, struct genl_info *info)
> > if (info->attrs[NL80211_ATTR_VLAN_ID])
> > params.vlan_id = nla_get_u16(info->attrs[NL80211_ATTR_VLAN_ID]);
> >
> > - if (info->attrs[NL80211_ATTR_STA_LISTEN_INTERVAL])
> > - params.listen_interval =
> > - nla_get_u16(info->attrs[NL80211_ATTR_STA_LISTEN_INTERVAL]);
> > - else
> > - params.listen_interval = -1;
> > + params.listen_interval = nla_get_u16_default(info->attrs[NL80211_ATTR_STA_LISTEN_INTERVAL],
> > + -1);
>
>
> this one clearly is.
Ouch, and really easy to miss while reviewing.
If nla_get_*_default was a macro (generating an "attr ? getter :
default" expression) you wouldn't have that problem I think, but you
couldn't nicely generate all the helpers with MAKE_NLA_GET_DEFAULT
anymore.
--
Sabrina
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH 2/2] net: convert to nla_get_*_default()
2024-10-24 14:48 ` Sabrina Dubroca
@ 2024-10-24 14:52 ` Johannes Berg
2024-10-24 15:17 ` Sabrina Dubroca
0 siblings, 1 reply; 17+ messages in thread
From: Johannes Berg @ 2024-10-24 14:52 UTC (permalink / raw)
To: Sabrina Dubroca; +Cc: linux-wireless, netdev
On Thu, 2024-10-24 at 16:48 +0200, Sabrina Dubroca wrote:
>
> If nla_get_*_default was a macro (generating an "attr ? getter :
> default" expression) you wouldn't have that problem I think,
Hmm. Perhaps. In the conditional operator (?:) they're subject to
integer promotion though, I wonder if that could cause some subtle issue
too especially if nla_get_u*() is used with signed variables?
> but you
> couldn't nicely generate all the helpers with MAKE_NLA_GET_DEFAULT
> anymore.
Right, that too.
I think it's probably better to just review them, and only commit the
obvious ones originally?
johannes
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH 2/2] net: convert to nla_get_*_default()
2024-10-24 14:52 ` Johannes Berg
@ 2024-10-24 15:17 ` Sabrina Dubroca
2024-10-24 15:29 ` Johannes Berg
0 siblings, 1 reply; 17+ messages in thread
From: Sabrina Dubroca @ 2024-10-24 15:17 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, netdev
2024-10-24, 16:52:00 +0200, Johannes Berg wrote:
> On Thu, 2024-10-24 at 16:48 +0200, Sabrina Dubroca wrote:
> >
> > If nla_get_*_default was a macro (generating an "attr ? getter :
> > default" expression) you wouldn't have that problem I think,
>
> Hmm. Perhaps. In the conditional operator (?:) they're subject to
> integer promotion though
Is it?
#define nla_get_u16_default(attr, d) (attr ? nla_get_u16(attr) : d)
int v = nla_get_u16_default(NULL, -1);
seems to put the correct value into v.
(but -ENOFOOD and -ELOWCOFFEE here, so I don't trust this quick test
much :))
>, I wonder if that could cause some subtle issue
> too especially if nla_get_u*() is used with signed variables?
The issue in that example is pretty subtle and I'm fairly sure people
are going to mess up :/
But I'm not attached to that macro I just suggested, it's just a
thought.
> > but you
> > couldn't nicely generate all the helpers with MAKE_NLA_GET_DEFAULT
> > anymore.
>
> Right, that too.
>
> I think it's probably better to just review them, and only commit the
> obvious ones originally?
Well, this one looked reasonable too. I'm not convinced reviewers are
going to catch those problems. Or authors of new code using those
_default helpers from the start.
--
Sabrina
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH 2/2] net: convert to nla_get_*_default()
2024-10-24 15:17 ` Sabrina Dubroca
@ 2024-10-24 15:29 ` Johannes Berg
0 siblings, 0 replies; 17+ messages in thread
From: Johannes Berg @ 2024-10-24 15:29 UTC (permalink / raw)
To: Sabrina Dubroca; +Cc: linux-wireless, netdev
On Thu, 2024-10-24 at 17:17 +0200, Sabrina Dubroca wrote:
> 2024-10-24, 16:52:00 +0200, Johannes Berg wrote:
> > On Thu, 2024-10-24 at 16:48 +0200, Sabrina Dubroca wrote:
> > >
> > > If nla_get_*_default was a macro (generating an "attr ? getter :
> > > default" expression) you wouldn't have that problem I think,
> >
> > Hmm. Perhaps. In the conditional operator (?:) they're subject to
> > integer promotion though
>
> Is it?
>
> #define nla_get_u16_default(attr, d) (attr ? nla_get_u16(attr) : d)
> int v = nla_get_u16_default(NULL, -1);
>
> seems to put the correct value into v.
> (but -ENOFOOD and -ELOWCOFFEE here, so I don't trust this quick test
> much :))
I'm probably -ELOWBLOODSUGAR right now ;-)
But yeah, I couldn't yet construct a scenario where it mattered, but I'
also couldn't convince myself that there isn't one. Maybe too much
inspired by the enum compare warnings:
https://lore.kernel.org/linux-wireless/20241018151841.3821671-1-arnd@kernel.org/
> > , I wonder if that could cause some subtle issue
> > too especially if nla_get_u*() is used with signed variables?
>
> The issue in that example is pretty subtle and I'm fairly sure people
> are going to mess up :/
I didn't think it was that bad, but it's well possible that my
calibration for "subtle" is way off ;-)
> But I'm not attached to that macro I just suggested, it's just a
> thought.
Sure.
> > > but you
> > > couldn't nicely generate all the helpers with MAKE_NLA_GET_DEFAULT
> > > anymore.
> >
> > Right, that too.
> >
> > I think it's probably better to just review them, and only commit the
> > obvious ones originally?
>
> Well, this one looked reasonable too. I'm not convinced reviewers are
> going to catch those problems. Or authors of new code using those
> _default helpers from the start.
Fair point. I didn't think it was that bad, in fact there were some I
was far less sure about, say
> + request->page = nla_get_u8_default(info->attrs[NL802154_ATTR_PAGE],
> + wpan_phy->current_page);
which really depends on the types of 'page' and 'current_page'...
I think for most cases it's probably still worth doing, and I wouldn't
be _too_ concerned about the type issues here, most places are just
using zero or a small constant as the default anyway.
Even nla_get_XX_or_zero() would be a win, but that seemed too special
...
johannes
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH 1/2] net: netlink: add nla_get_*_default() accessors
2024-10-24 11:18 [RFC PATCH 1/2] net: netlink: add nla_get_*_default() accessors Johannes Berg
` (2 preceding siblings ...)
2024-10-24 12:51 ` Toke Høiland-Jørgensen
@ 2024-10-24 15:42 ` kernel test robot
2024-10-24 16:35 ` kernel test robot
2024-10-29 15:54 ` Jakub Kicinski
5 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2024-10-24 15:42 UTC (permalink / raw)
To: Johannes Berg; +Cc: oe-kbuild-all
Hi Johannes,
[This is a private test report for your RFC patch.]
kernel test robot noticed the following build errors:
[auto build test ERROR on wireless-next/main]
[also build test ERROR on wireless/main netfilter-nf/main linus/master horms-ipvs/master nf-next/master v6.12-rc4 next-20241024]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Johannes-Berg/net-convert-to-nla_get_-_default/20241024-191943
base: https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main
patch link: https://lore.kernel.org/r/20241024131807.0a6c07355832.I3df6aac71d38a5baa1c0a03d0c7e82d4395c030e%40changeid
patch subject: [RFC PATCH 1/2] net: netlink: add nla_get_*_default() accessors
config: x86_64-rhel-8.3 (https://download.01.org/0day-ci/archive/20241024/202410242345.2HEJkySu-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241024/202410242345.2HEJkySu-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410242345.2HEJkySu-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from include/net/rtnetlink.h:6,
from include/net/neighbour.h:31,
from include/net/dst.h:20,
from include/net/sock.h:66,
from include/linux/tcp.h:19,
from include/linux/ipv6.h:102,
from include/net/gro.h:8,
from net/8021q/vlan_core.c:7:
include/net/netlink.h: In function 'nla_get_u8_default':
>> include/net/netlink.h:1878:16: error: implicit declaration of function 'n' [-Werror=implicit-function-declaration]
1878 | return n(nla); \
| ^
include/net/netlink.h:1881:1: note: in expansion of macro 'MAKE_NLA_GET_DEFAULT'
1881 | MAKE_NLA_GET_DEFAULT(u8, nla_get_u8);
| ^~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
--
In file included from include/net/rtnetlink.h:6,
from include/net/neighbour.h:31,
from include/net/arp.h:8,
from net/8021q/vlan.c:27:
include/net/netlink.h: In function 'nla_get_u8_default':
>> include/net/netlink.h:1878:16: error: implicit declaration of function 'n' [-Werror=implicit-function-declaration]
1878 | return n(nla); \
| ^
include/net/netlink.h:1881:1: note: in expansion of macro 'MAKE_NLA_GET_DEFAULT'
1881 | MAKE_NLA_GET_DEFAULT(u8, nla_get_u8);
| ^~~~~~~~~~~~~~~~~~~~
net/8021q/vlan.c: In function 'register_vlan_device':
net/8021q/vlan.c:247:46: warning: '%i' directive output may be truncated writing between 1 and 4 bytes into a region of size between 0 and 15 [-Wformat-truncation=]
247 | snprintf(name, IFNAMSIZ, "%s.%i", real_dev->name, vlan_id);
| ^~
net/8021q/vlan.c:247:42: note: directive argument in the range [0, 4094]
247 | snprintf(name, IFNAMSIZ, "%s.%i", real_dev->name, vlan_id);
| ^~~~~~~
net/8021q/vlan.c:247:17: note: 'snprintf' output between 3 and 21 bytes into a destination of size 16
247 | snprintf(name, IFNAMSIZ, "%s.%i", real_dev->name, vlan_id);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
net/8021q/vlan.c:235:46: warning: '%.4i' directive output may be truncated writing 4 bytes into a region of size between 0 and 15 [-Wformat-truncation=]
235 | snprintf(name, IFNAMSIZ, "%s.%.4i", real_dev->name, vlan_id);
| ^~~~
net/8021q/vlan.c:235:42: note: directive argument in the range [0, 4094]
235 | snprintf(name, IFNAMSIZ, "%s.%.4i", real_dev->name, vlan_id);
| ^~~~~~~~~
net/8021q/vlan.c:235:17: note: 'snprintf' output between 6 and 21 bytes into a destination of size 16
235 | snprintf(name, IFNAMSIZ, "%s.%.4i", real_dev->name, vlan_id);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/n +1878 include/net/netlink.h
1871
1872 #define MAKE_NLA_GET_DEFAULT(tp, fn) \
1873 static inline tp fn##_default(const struct nlattr *nla, \
1874 tp defvalue) \
1875 { \
1876 if (!nla) \
1877 return defvalue; \
> 1878 return n(nla); \
1879 }
1880
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH 1/2] net: netlink: add nla_get_*_default() accessors
2024-10-24 11:18 [RFC PATCH 1/2] net: netlink: add nla_get_*_default() accessors Johannes Berg
` (3 preceding siblings ...)
2024-10-24 15:42 ` kernel test robot
@ 2024-10-24 16:35 ` kernel test robot
2024-10-29 15:54 ` Jakub Kicinski
5 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2024-10-24 16:35 UTC (permalink / raw)
To: Johannes Berg; +Cc: llvm, oe-kbuild-all
Hi Johannes,
[This is a private test report for your RFC patch.]
kernel test robot noticed the following build errors:
[auto build test ERROR on wireless-next/main]
[also build test ERROR on wireless/main netfilter-nf/main linus/master horms-ipvs/master nf-next/master v6.12-rc4 next-20241024]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Johannes-Berg/net-convert-to-nla_get_-_default/20241024-191943
base: https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main
patch link: https://lore.kernel.org/r/20241024131807.0a6c07355832.I3df6aac71d38a5baa1c0a03d0c7e82d4395c030e%40changeid
patch subject: [RFC PATCH 1/2] net: netlink: add nla_get_*_default() accessors
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20241025/202410250052.PY6qSKo7-lkp@intel.com/config)
compiler: clang version 19.1.2 (https://github.com/llvm/llvm-project 7ba7d8e2f7b6445b60679da826210cdde29eaf8b)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241025/202410250052.PY6qSKo7-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410250052.PY6qSKo7-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
In file included from fs/nfs/blocklayout/blocklayout.c:37:
In file included from include/linux/bio.h:10:
In file included from include/linux/blk_types.h:10:
In file included from include/linux/bvec.h:10:
In file included from include/linux/highmem.h:8:
In file included from include/linux/cacheflush.h:5:
In file included from arch/x86/include/asm/cacheflush.h:5:
In file included from include/linux/mm.h:2213:
include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
504 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
505 | item];
| ~~~~
include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
511 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
512 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
518 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
524 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
525 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
In file included from fs/nfs/blocklayout/blocklayout.c:41:
In file included from fs/nfs/blocklayout/../pnfs.h:34:
In file included from include/linux/nfs_fs.h:32:
In file included from include/linux/sunrpc/clnt.h:29:
In file included from include/net/ipv6.h:12:
In file included from include/linux/ipv6.h:102:
In file included from include/linux/tcp.h:19:
In file included from include/net/sock.h:66:
In file included from include/net/dst.h:20:
In file included from include/net/neighbour.h:31:
In file included from include/net/rtnetlink.h:6:
>> include/net/netlink.h:1881:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1881 | MAKE_NLA_GET_DEFAULT(u8, nla_get_u8);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1882:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1882 | MAKE_NLA_GET_DEFAULT(u16, nla_get_u16);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1883:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1883 | MAKE_NLA_GET_DEFAULT(u32, nla_get_u32);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1884:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1884 | MAKE_NLA_GET_DEFAULT(u64, nla_get_u64);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1885:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1885 | MAKE_NLA_GET_DEFAULT(unsigned long, nla_get_msecs);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1886:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1886 | MAKE_NLA_GET_DEFAULT(s8, nla_get_s8);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1887:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1887 | MAKE_NLA_GET_DEFAULT(s16, nla_get_s16);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1888:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1888 | MAKE_NLA_GET_DEFAULT(s32, nla_get_s32);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1889:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1889 | MAKE_NLA_GET_DEFAULT(s64, nla_get_s64);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1890:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1890 | MAKE_NLA_GET_DEFAULT(s16, nla_get_le16);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1891:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1891 | MAKE_NLA_GET_DEFAULT(s32, nla_get_le32);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1892:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1892 | MAKE_NLA_GET_DEFAULT(s64, nla_get_le64);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1893:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1893 | MAKE_NLA_GET_DEFAULT(s16, nla_get_be16);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1894:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1894 | MAKE_NLA_GET_DEFAULT(s32, nla_get_be32);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1895:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1895 | MAKE_NLA_GET_DEFAULT(s64, nla_get_be64);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
fs/nfs/blocklayout/blocklayout.c:384:9: warning: variable 'count' set but not used [-Wunused-but-set-variable]
384 | size_t count = header->args.count;
| ^
5 warnings and 15 errors generated.
--
In file included from fs/nfs/blocklayout/dev.c:5:
In file included from include/linux/sunrpc/svc.h:17:
In file included from include/linux/sunrpc/xdr.h:17:
In file included from include/linux/scatterlist.h:8:
In file included from include/linux/mm.h:2213:
include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
504 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
505 | item];
| ~~~~
include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
511 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
512 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
518 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
524 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
525 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
In file included from fs/nfs/blocklayout/dev.c:5:
In file included from include/linux/sunrpc/svc.h:19:
In file included from include/linux/sunrpc/svcauth.h:17:
In file included from include/linux/sunrpc/clnt.h:29:
In file included from include/net/ipv6.h:12:
In file included from include/linux/ipv6.h:102:
In file included from include/linux/tcp.h:19:
In file included from include/net/sock.h:66:
In file included from include/net/dst.h:20:
In file included from include/net/neighbour.h:31:
In file included from include/net/rtnetlink.h:6:
>> include/net/netlink.h:1881:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1881 | MAKE_NLA_GET_DEFAULT(u8, nla_get_u8);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1882:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1882 | MAKE_NLA_GET_DEFAULT(u16, nla_get_u16);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1883:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1883 | MAKE_NLA_GET_DEFAULT(u32, nla_get_u32);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1884:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1884 | MAKE_NLA_GET_DEFAULT(u64, nla_get_u64);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1885:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1885 | MAKE_NLA_GET_DEFAULT(unsigned long, nla_get_msecs);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1886:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1886 | MAKE_NLA_GET_DEFAULT(s8, nla_get_s8);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1887:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1887 | MAKE_NLA_GET_DEFAULT(s16, nla_get_s16);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1888:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1888 | MAKE_NLA_GET_DEFAULT(s32, nla_get_s32);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1889:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1889 | MAKE_NLA_GET_DEFAULT(s64, nla_get_s64);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1890:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1890 | MAKE_NLA_GET_DEFAULT(s16, nla_get_le16);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1891:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1891 | MAKE_NLA_GET_DEFAULT(s32, nla_get_le32);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1892:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1892 | MAKE_NLA_GET_DEFAULT(s64, nla_get_le64);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1893:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1893 | MAKE_NLA_GET_DEFAULT(s16, nla_get_be16);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1894:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1894 | MAKE_NLA_GET_DEFAULT(s32, nla_get_be32);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1895:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1895 | MAKE_NLA_GET_DEFAULT(s64, nla_get_be64);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
4 warnings and 15 errors generated.
--
In file included from drivers/net/ethernet/intel/e1000e/netdev.c:9:
In file included from include/linux/pci.h:1650:
In file included from include/linux/dmapool.h:14:
In file included from include/linux/scatterlist.h:8:
In file included from include/linux/mm.h:2213:
include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
504 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
505 | item];
| ~~~~
include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
511 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
512 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
518 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
524 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
525 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
In file included from drivers/net/ethernet/intel/e1000e/netdev.c:15:
In file included from include/linux/tcp.h:19:
In file included from include/net/sock.h:66:
In file included from include/net/dst.h:20:
In file included from include/net/neighbour.h:31:
In file included from include/net/rtnetlink.h:6:
>> include/net/netlink.h:1881:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1881 | MAKE_NLA_GET_DEFAULT(u8, nla_get_u8);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1882:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1882 | MAKE_NLA_GET_DEFAULT(u16, nla_get_u16);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1883:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1883 | MAKE_NLA_GET_DEFAULT(u32, nla_get_u32);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1884:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1884 | MAKE_NLA_GET_DEFAULT(u64, nla_get_u64);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1885:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1885 | MAKE_NLA_GET_DEFAULT(unsigned long, nla_get_msecs);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1886:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1886 | MAKE_NLA_GET_DEFAULT(s8, nla_get_s8);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1887:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1887 | MAKE_NLA_GET_DEFAULT(s16, nla_get_s16);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1888:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1888 | MAKE_NLA_GET_DEFAULT(s32, nla_get_s32);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1889:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1889 | MAKE_NLA_GET_DEFAULT(s64, nla_get_s64);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1890:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1890 | MAKE_NLA_GET_DEFAULT(s16, nla_get_le16);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1891:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1891 | MAKE_NLA_GET_DEFAULT(s32, nla_get_le32);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1892:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1892 | MAKE_NLA_GET_DEFAULT(s64, nla_get_le64);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1893:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1893 | MAKE_NLA_GET_DEFAULT(s16, nla_get_be16);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1894:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1894 | MAKE_NLA_GET_DEFAULT(s32, nla_get_be32);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
include/net/netlink.h:1895:1: error: call to undeclared function 'n'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
1895 | MAKE_NLA_GET_DEFAULT(s64, nla_get_be64);
| ^
include/net/netlink.h:1878:9: note: expanded from macro 'MAKE_NLA_GET_DEFAULT'
1878 | return n(nla); \
| ^
>> drivers/net/ethernet/intel/e1000e/netdev.c:4460:22: warning: shift count >= width of type [-Wshift-count-overflow]
4460 | adapter->cc.mask = CYCLECOUNTER_MASK(64);
| ^~~~~~~~~~~~~~~~~~~~~
include/linux/timecounter.h:14:59: note: expanded from macro 'CYCLECOUNTER_MASK'
14 | #define CYCLECOUNTER_MASK(bits) (u64)((bits) < 64 ? ((1ULL<<(bits))-1) : -1)
| ^ ~~~~~~
drivers/net/ethernet/intel/e1000e/netdev.c:7386:46: warning: shift count >= width of type [-Wshift-count-overflow]
7386 | err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
| ^~~~~~~~~~~~~~~~
include/linux/dma-mapping.h:77:54: note: expanded from macro 'DMA_BIT_MASK'
77 | #define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
| ^ ~~~
6 warnings and 15 errors generated.
..
vim +/n +1881 include/net/netlink.h
1880
> 1881 MAKE_NLA_GET_DEFAULT(u8, nla_get_u8);
1882 MAKE_NLA_GET_DEFAULT(u16, nla_get_u16);
1883 MAKE_NLA_GET_DEFAULT(u32, nla_get_u32);
1884 MAKE_NLA_GET_DEFAULT(u64, nla_get_u64);
1885 MAKE_NLA_GET_DEFAULT(unsigned long, nla_get_msecs);
1886 MAKE_NLA_GET_DEFAULT(s8, nla_get_s8);
1887 MAKE_NLA_GET_DEFAULT(s16, nla_get_s16);
1888 MAKE_NLA_GET_DEFAULT(s32, nla_get_s32);
1889 MAKE_NLA_GET_DEFAULT(s64, nla_get_s64);
1890 MAKE_NLA_GET_DEFAULT(s16, nla_get_le16);
1891 MAKE_NLA_GET_DEFAULT(s32, nla_get_le32);
1892 MAKE_NLA_GET_DEFAULT(s64, nla_get_le64);
1893 MAKE_NLA_GET_DEFAULT(s16, nla_get_be16);
1894 MAKE_NLA_GET_DEFAULT(s32, nla_get_be32);
1895 MAKE_NLA_GET_DEFAULT(s64, nla_get_be64);
1896
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH 1/2] net: netlink: add nla_get_*_default() accessors
2024-10-24 11:18 [RFC PATCH 1/2] net: netlink: add nla_get_*_default() accessors Johannes Berg
` (4 preceding siblings ...)
2024-10-24 16:35 ` kernel test robot
@ 2024-10-29 15:54 ` Jakub Kicinski
5 siblings, 0 replies; 17+ messages in thread
From: Jakub Kicinski @ 2024-10-29 15:54 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, netdev, Johannes Berg
On Thu, 24 Oct 2024 13:18:06 +0200 Johannes Berg wrote:
> +#define MAKE_NLA_GET_DEFAULT(tp, fn) \
> +static inline tp fn##_default(const struct nlattr *nla, \
> + tp defvalue) \
> +{ \
> + if (!nla) \
> + return defvalue; \
> + return n(nla); \
> +}
> +
> +MAKE_NLA_GET_DEFAULT(u8, nla_get_u8);
> +MAKE_NLA_GET_DEFAULT(u16, nla_get_u16);
> +MAKE_NLA_GET_DEFAULT(u32, nla_get_u32);
> +MAKE_NLA_GET_DEFAULT(u64, nla_get_u64);
> +MAKE_NLA_GET_DEFAULT(unsigned long, nla_get_msecs);
> +MAKE_NLA_GET_DEFAULT(s8, nla_get_s8);
> +MAKE_NLA_GET_DEFAULT(s16, nla_get_s16);
> +MAKE_NLA_GET_DEFAULT(s32, nla_get_s32);
> +MAKE_NLA_GET_DEFAULT(s64, nla_get_s64);
> +MAKE_NLA_GET_DEFAULT(s16, nla_get_le16);
> +MAKE_NLA_GET_DEFAULT(s32, nla_get_le32);
> +MAKE_NLA_GET_DEFAULT(s64, nla_get_le64);
> +MAKE_NLA_GET_DEFAULT(s16, nla_get_be16);
> +MAKE_NLA_GET_DEFAULT(s32, nla_get_be32);
> +MAKE_NLA_GET_DEFAULT(s64, nla_get_be64);
I'd vote to just spell out the accessors instead of hinding
the definitions behind macros. Place them right after the
existing nla_get_* definition to make it more likely people
will notice them.
Either way:
Acked-by: Jakub Kicinski <kuba@kernel.org>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH 2/2] net: convert to nla_get_*_default()
2024-10-24 11:18 ` [RFC PATCH 2/2] net: convert to nla_get_*_default() Johannes Berg
2024-10-24 12:11 ` Johannes Berg
2024-10-24 13:41 ` Johannes Berg
@ 2024-10-29 15:57 ` Jakub Kicinski
2 siblings, 0 replies; 17+ messages in thread
From: Jakub Kicinski @ 2024-10-29 15:57 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, netdev, Johannes Berg
On Thu, 24 Oct 2024 13:18:07 +0200 Johannes Berg wrote:
> diff --git a/net/sched/sch_gred.c b/net/sched/sch_gred.c
> index 79ba9dc70254..7a10bb159113 100644
> --- a/net/sched/sch_gred.c
> +++ b/net/sched/sch_gred.c
> @@ -750,11 +750,8 @@ static int gred_init(struct Qdisc *sch, struct nlattr *opt,
> return -EINVAL;
> }
>
> - if (tb[TCA_GRED_LIMIT])
> - sch->limit = nla_get_u32(tb[TCA_GRED_LIMIT]);
> - else
> - sch->limit = qdisc_dev(sch)->tx_queue_len
> - * psched_mtu(qdisc_dev(sch));
> + sch->limit = nla_get_u32_default(tb[TCA_GRED_LIMIT],
> + qdisc_dev(sch)->tx_queue_len * psched_mtu(qdisc_dev(sch)));
> @@ -7578,10 +7569,8 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
> dev->ieee80211_ptr->iftype == NL80211_IFTYPE_P2P_GO;
> }
>
> - if (info->attrs[NL80211_ATTR_PEER_AID])
> - params.aid = nla_get_u16(info->attrs[NL80211_ATTR_PEER_AID]);
> - else
> - params.aid = nla_get_u16(info->attrs[NL80211_ATTR_STA_AID]);
> + params.aid = nla_get_u16_default(info->attrs[NL80211_ATTR_PEER_AID],
> + nla_get_u16(info->attrs[NL80211_ATTR_STA_AID]));
I'd limit the conversions only to cases where the default is a constant.
In the two cases quoted here it seems like the conversion results in net
loss of readability.
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2024-10-29 15:57 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-24 11:18 [RFC PATCH 1/2] net: netlink: add nla_get_*_default() accessors Johannes Berg
2024-10-24 11:18 ` [RFC PATCH 2/2] net: convert to nla_get_*_default() Johannes Berg
2024-10-24 12:11 ` Johannes Berg
2024-10-24 14:28 ` Sabrina Dubroca
2024-10-24 14:31 ` Johannes Berg
2024-10-24 14:40 ` Johannes Berg
2024-10-24 13:41 ` Johannes Berg
2024-10-24 14:48 ` Sabrina Dubroca
2024-10-24 14:52 ` Johannes Berg
2024-10-24 15:17 ` Sabrina Dubroca
2024-10-24 15:29 ` Johannes Berg
2024-10-29 15:57 ` Jakub Kicinski
2024-10-24 11:29 ` [RFC PATCH 1/2] net: netlink: add nla_get_*_default() accessors Johannes Berg
2024-10-24 12:51 ` Toke Høiland-Jørgensen
2024-10-24 15:42 ` kernel test robot
2024-10-24 16:35 ` kernel test robot
2024-10-29 15:54 ` Jakub Kicinski
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.