Re: [PATCH rfc v0 6/9] net: dsa: qca8k: Refactor sequence number mismatch to use error code
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Date: 2022-09-19 23:31:07
On Tue, Sep 20, 2022 at 12:18:50AM +0200, Andrew Lunn wrote:
quoted hunk ↗ jump to hunk
@@ -229,7 +231,7 @@ static int qca8k_read_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len) { struct qca8k_mgmt_eth_data *mgmt_eth_data = &priv->mgmt_eth_data; struct sk_buff *skb; - bool ack; + int err; int ret; skb = qca8k_alloc_mdio_header(MDIO_READ, reg, NULL,@@ -247,7 +249,6 @@ static int qca8k_read_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len) } skb->dev = priv->mgmt_master; - mgmt_eth_data->ack = false; ret = dsa_inband_request(&mgmt_eth_data->inband, skb, qca8k_mdio_header_fill_seq_num,@@ -257,15 +258,15 @@ static int qca8k_read_eth(struct qca8k_priv *priv, u32 reg, u32 *val, int len) if (len > QCA_HDR_MGMT_DATA1_LEN) memcpy(val + 1, mgmt_eth_data->data + 1, len - QCA_HDR_MGMT_DATA1_LEN); - ack = mgmt_eth_data->ack; + err = mgmt_eth_data->err; mutex_unlock(&mgmt_eth_data->mutex); if (ret) return ret; - if (!ack) - return -EINVAL; + if (err) + return -ret;
Probably "if (err) return -ret" is not what you intend. We know ret is 0, we just checked for it earlier. Also, maybe a variable named "match" would be more expressive? This shows how easy it is to make mistakes, mixing "err" with "ret" in the same function.
return 0;
Can it actually be expressed as "return err", here and everywhere else?
}