lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sun, 31 Mar 2019 16:45:48 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Heiner Kallweit <hkallweit1@...il.com>
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        David Miller <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] net: phy: aquantia: add SGMII statistics

> +static u64 aqr107_get_stat(struct phy_device *phydev, int index)
> +{
> +	const struct aqr107_hw_stat *stat = aqr107_hw_stats + index;
> +	int len_l = min(stat->size, 16);
> +	int len_h = stat->size - len_l;
> +	u64 ret;
> +	int val;
> +
> +	val = phy_read_mmd(phydev, MDIO_MMD_C22EXT, stat->reg);
> +	if (val < 0) {
> +		phydev_err(phydev, "Reading HW Statistics failed\n");
> +		return 0;
> +	}
> +
> +	ret = val & GENMASK(len_l - 1, 0);
> +	if (len_h) {
> +		val = phy_read_mmd(phydev, MDIO_MMD_C22EXT, stat->reg + 1);
> +		if (val < 0) {
> +			phydev_err(phydev, "Reading HW Statistics failed\n");
> +			return 0;

Hi Heiner

When things go wrong, it seems to be reasonably normal to return
U64_MAX, not zero. It is such a large value that is raises questions,
where as 0 might be considered a correct value, not an error.

> +static void aqr107_get_stats(struct phy_device *phydev,
> +			     struct ethtool_stats *stats, u64 *data)
> +{
> +	u64 *pstats = phydev->priv;

This seems like a trap waiting for somebody to fall into.

It would be more future proof to define a struct which just contains
an array. 

   Andrew

Powered by blists - more mailing lists