On Mon, 2018-08-27 at 12:16 -0400, rpj...@crashcourse.ca wrote:
> Quoting David Miller <da...@davemloft.net>:
>
> > From: "Robert P. J. Day" <rpj...@crashcourse.ca>
> > Date: Mon, 27 Aug 2018 04:55:29 -0400 (EDT)
> >
> > > another pedantic oddity -- is there a reason for these two double
> > > negations in net/core/net-sysfs.c?
> >
> > It turns an arbitrary integer into a boolean, this is a common
> > construct across the kernel tree so I'm surprised you've never seen
> > it before.
> >
> > Although, I don't know how much more hand holding we're willing to
> > tolerate continuing to give to you at this point.
> >
> > Thanks.
>
> I mentioned in my earlier email that I know what that construct is
> *typically* used for; I also pointed out that, AFAICT, it was totally
> unnecessary in the context of the two routines I mentioned, which
> would appear to ever return only one of two boolean values, 0 or 1.
Both are bool functions, so it's guaranteed and the
!! uses are redundant.
include/linux/netdevice.h:static inline bool netif_carrier_ok(const struct
net_device *dev)
include/linux/netdevice.h:static inline bool netif_dormant(const struct
net_device *dev)
And there are 4 uses with !!
$ git grep -P '\!\s*\!\s*netif_(?:dormant|carrier_ok)\s*\('
drivers/net/team/team.c: __team_port_change_port_added(port,
!!netif_carrier_ok(port_dev));
drivers/net/usb/r8152.c: bool sw_linking =
!!netif_carrier_ok(tp->netdev);
net/core/net-sysfs.c: return sprintf(buf, fmt_dec,
!!netif_carrier_ok(netdev));
net/core/net-sysfs.c: return sprintf(buf, fmt_dec,
!!netif_dormant(netdev));
out of the ~400 or so treewide.
Only redeeming use of the !! is the fmt_dec expects
an int and this forces the type to int, though the
compiler would do that automatically anyway.
I'd suggest removing the fmt_<foo> uses for clarity.
---
net/core/net-sysfs.c | 49 ++++++++++++++++++++++---------------------------
1 file changed, 22 insertions(+), 27 deletions(-)
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index bd67c4d0fcfd..ecaa684f4b92 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -31,11 +31,6 @@
#include "net-sysfs.h"
#ifdef CONFIG_SYSFS
-static const char fmt_hex[] = "%#x\n";
-static const char fmt_dec[] = "%d\n";
-static const char fmt_ulong[] = "%lu\n";
-static const char fmt_u64[] = "%llu\n";
-
static inline int dev_isalive(const struct net_device *dev)
{
return dev->reg_state <= NETREG_REGISTERED;
@@ -107,26 +102,26 @@ static ssize_t netdev_store(struct device *dev, struct
device_attribute *attr,
return ret;
}
-NETDEVICE_SHOW_RO(dev_id, fmt_hex);
-NETDEVICE_SHOW_RO(dev_port, fmt_dec);
-NETDEVICE_SHOW_RO(addr_assign_type, fmt_dec);
-NETDEVICE_SHOW_RO(addr_len, fmt_dec);
-NETDEVICE_SHOW_RO(ifindex, fmt_dec);
-NETDEVICE_SHOW_RO(type, fmt_dec);
-NETDEVICE_SHOW_RO(link_mode, fmt_dec);
+NETDEVICE_SHOW_RO(dev_id, "%#x\n");
+NETDEVICE_SHOW_RO(dev_port, "%d\n");
+NETDEVICE_SHOW_RO(addr_assign_type, "%d\n");
+NETDEVICE_SHOW_RO(addr_len, "%d\n");
+NETDEVICE_SHOW_RO(ifindex, "%d\n");
+NETDEVICE_SHOW_RO(type, "%d\n");
+NETDEVICE_SHOW_RO(link_mode, "%d\n");
static ssize_t iflink_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
struct net_device *ndev = to_net_dev(dev);
- return sprintf(buf, fmt_dec, dev_get_iflink(ndev));
+ return sprintf(buf, "%d\n", dev_get_iflink(ndev));
}
static DEVICE_ATTR_RO(iflink);
static ssize_t format_name_assign_type(const struct net_device *dev, char *buf)
{
- return sprintf(buf, fmt_dec, dev->name_assign_type);
+ return sprintf(buf, "%d\n", dev->name_assign_type);
}
static ssize_t name_assign_type_show(struct device *dev,
@@ -188,7 +183,7 @@ static ssize_t carrier_show(struct device *dev,
struct net_device *netdev = to_net_dev(dev);
if (netif_running(netdev))
- return sprintf(buf, fmt_dec, !!netif_carrier_ok(netdev));
+ return sprintf(buf, "%d\n", !!netif_carrier_ok(netdev));
return -EINVAL;
}
@@ -207,7 +202,7 @@ static ssize_t speed_show(struct device *dev,
struct ethtool_link_ksettings cmd;
if (!__ethtool_get_link_ksettings(netdev, &cmd))
- ret = sprintf(buf, fmt_dec, cmd.base.speed);
+ ret = sprintf(buf, "%d\n", cmd.base.speed);
}
rtnl_unlock();
return ret;
@@ -254,7 +249,7 @@ static ssize_t dormant_show(struct device *dev,
struct net_device *netdev = to_net_dev(dev);
if (netif_running(netdev))
- return sprintf(buf, fmt_dec, !!netif_dormant(netdev));
+ return sprintf(buf, "%d\n", !!netif_dormant(netdev));
return -EINVAL;
}
@@ -295,7 +290,7 @@ static ssize_t carrier_changes_show(struct device *dev,
{
struct net_device *netdev = to_net_dev(dev);
- return sprintf(buf, fmt_dec,
+ return sprintf(buf, "%d\n",
atomic_read(&netdev->carrier_up_count) +
atomic_read(&netdev->carrier_down_count));
}
@@ -307,7 +302,7 @@ static ssize_t carrier_up_count_show(struct device *dev,
{
struct net_device *netdev = to_net_dev(dev);
- return sprintf(buf, fmt_dec, atomic_read(&netdev->carrier_up_count));
+ return sprintf(buf, "%d\n", atomic_read(&netdev->carrier_up_count));
}
static DEVICE_ATTR_RO(carrier_up_count);
@@ -317,7 +312,7 @@ static ssize_t carrier_down_count_show(struct device *dev,
{
struct net_device *netdev = to_net_dev(dev);
- return sprintf(buf, fmt_dec, atomic_read(&netdev->carrier_down_count));
+ return sprintf(buf, "%d\n", atomic_read(&netdev->carrier_down_count));
}
static DEVICE_ATTR_RO(carrier_down_count);
@@ -333,7 +328,7 @@ static ssize_t mtu_store(struct device *dev, struct
device_attribute *attr,
{
return netdev_store(dev, attr, buf, len, change_mtu);
}
-NETDEVICE_SHOW_RW(mtu, fmt_dec);
+NETDEVICE_SHOW_RW(mtu, "%d\n");
static int change_flags(struct net_device *dev, unsigned long new_flags)
{
@@ -345,7 +340,7 @@ static ssize_t flags_store(struct device *dev, struct
device_attribute *attr,
{
return netdev_store(dev, attr, buf, len, change_flags);
}
-NETDEVICE_SHOW_RW(flags, fmt_hex);
+NETDEVICE_SHOW_RW(flags, "%#x\n");
static ssize_t tx_queue_len_store(struct device *dev,
struct device_attribute *attr,
@@ -356,7 +351,7 @@ static ssize_t tx_queue_len_store(struct device *dev,
return netdev_store(dev, attr, buf, len, dev_change_tx_queue_len);
}
-NETDEVICE_SHOW_RW(tx_queue_len, fmt_dec);
+NETDEVICE_SHOW_RW(tx_queue_len, "%d\n");
static int change_gro_flush_timeout(struct net_device *dev, unsigned long val)
{
@@ -373,7 +368,7 @@ static ssize_t gro_flush_timeout_store(struct device *dev,
return netdev_store(dev, attr, buf, len, change_gro_flush_timeout);
}
-NETDEVICE_SHOW_RW(gro_flush_timeout, fmt_ulong);
+NETDEVICE_SHOW_RW(gro_flush_timeout, "%lu\n");
static ssize_t ifalias_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t len)
@@ -431,7 +426,7 @@ static ssize_t group_store(struct device *dev, struct
device_attribute *attr,
{
return netdev_store(dev, attr, buf, len, change_group);
}
-NETDEVICE_SHOW(group, fmt_dec);
+NETDEVICE_SHOW(group, "%d\n");
static DEVICE_ATTR(netdev_group, 0644, group_show, group_store);
static int change_proto_down(struct net_device *dev, unsigned long proto_down)
@@ -445,7 +440,7 @@ static ssize_t proto_down_store(struct device *dev,
{
return netdev_store(dev, attr, buf, len, change_proto_down);
}
-NETDEVICE_SHOW_RW(proto_down, fmt_dec);
+NETDEVICE_SHOW_RW(proto_down, "%d\n");
static ssize_t phys_port_id_show(struct device *dev,
struct device_attribute *attr, char *buf)
@@ -568,7 +563,7 @@ static ssize_t netstat_show(const struct device *d,
struct rtnl_link_stats64 temp;
const struct rtnl_link_stats64 *stats = dev_get_stats(dev,
&temp);
- ret = sprintf(buf, fmt_u64, *(u64 *)(((u8 *)stats) + offset));
+ ret = sprintf(buf, "%llu\n", *(u64 *)(((u8 *)stats) + offset));
}
read_unlock(&dev_base_lock);
return ret;
