Re: [PATCH 1/4] Staging: ks7010: sdio: Fix multiple use of arguments in RX/TX queue macros.

[Dan Carpenter]

On Fri, Feb 16, 2018 at 06:20:50PM -0800, Quytelda Kahja wrote:
> Use GCC extensions to prevent macro arguments from accidentally being 
> evaluated
> multiple times when the macro is called.
> 

Don't over engineer code.  These bugs are never going to occur in real
life because of how the macros are used.

It should be pretty easy to make these into functions anyway...  Which
is a much simpler, better solution.

regards,
dan carpenter

Re: [PATCH 1/4] Staging: ks7010: sdio: Fix multiple use of arguments in RX/TX queue macros.

[Dan Carpenter]

On Fri, Feb 16, 2018 at 06:20:50PM -0800, Quytelda Kahja wrote:
> Use GCC extensions to prevent macro arguments from accidentally being 
> evaluated
> multiple times when the macro is called.
> 

Don't over engineer code.  These bugs are never going to occur in real
life because of how the macros are used.

It should be pretty easy to make these into functions anyway...  Which
is a much simpler, better solution.

regards,
dan carpenter

[PATCH 1/4] Staging: ks7010: sdio: Fix multiple use of arguments in RX/TX queue macros.

[Quytelda Kahja]

Use GCC extensions to prevent macro arguments from accidentally being evaluated
multiple times when the macro is called.

Signed-off-by: Quytelda Kahja 
---
 drivers/staging/ks7010/ks7010_sdio.c | 40 
 1 file changed, 27 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c 
b/drivers/staging/ks7010/ks7010_sdio.c
index 8cfdff198334..ffa7e2382353 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -32,19 +32,33 @@ static const struct sdio_device_id ks7010_sdio_ids[] = {
 };
 MODULE_DEVICE_TABLE(sdio, ks7010_sdio_ids);
 
-#define inc_txqhead(priv) \
-   (priv->tx_dev.qhead = (priv->tx_dev.qhead + 1) % TX_DEVICE_BUFF_SIZE)
-#define inc_txqtail(priv) \
-   (priv->tx_dev.qtail = (priv->tx_dev.qtail + 1) % TX_DEVICE_BUFF_SIZE)
-#define cnt_txqbody(priv) \
-   (((priv->tx_dev.qtail + TX_DEVICE_BUFF_SIZE) - (priv->tx_dev.qhead)) % 
TX_DEVICE_BUFF_SIZE)
-
-#define inc_rxqhead(priv) \
-   (priv->rx_dev.qhead = (priv->rx_dev.qhead + 1) % RX_DEVICE_BUFF_SIZE)
-#define inc_rxqtail(priv) \
-   (priv->rx_dev.qtail = (priv->rx_dev.qtail + 1) % RX_DEVICE_BUFF_SIZE)
-#define cnt_rxqbody(priv) \
-   (((priv->rx_dev.qtail + RX_DEVICE_BUFF_SIZE) - (priv->rx_dev.qhead)) % 
RX_DEVICE_BUFF_SIZE)
+#define inc_txqhead(priv)  \
+   ({ typeof(priv) priv_ = (priv); \
+   unsigned int next_qhead = priv_->tx_dev.qhead + 1;  \
+   priv_->tx_dev.qhead = next_qhead % TX_DEVICE_BUFF_SIZE; })
+#define inc_txqtail(priv)  \
+   ({ typeof(priv) priv_ = (priv); \
+   unsigned int next_qtail = priv_->tx_dev.qtail + 1;  \
+   priv_->tx_dev.qtail = next_qtail % TX_DEVICE_BUFF_SIZE; })
+#define cnt_txqbody(priv)  \
+   ({ typeof(priv) priv_ = (priv); \
+   unsigned int left_cnt = \
+   priv_->tx_dev.qtail + TX_DEVICE_BUFF_SIZE;  \
+   (left_cnt - (priv_->tx_dev.qhead)) % TX_DEVICE_BUFF_SIZE; })
+
+#define inc_rxqhead(priv)  \
+   ({ typeof(priv) priv_ = (priv); \
+   unsigned int next_qhead = priv_->rx_dev.qhead + 1;  \
+   priv_->rx_dev.qhead = next_qhead % RX_DEVICE_BUFF_SIZE; })
+#define inc_rxqtail(priv)  \
+   ({ typeof(priv) priv_ = (priv); \
+   unsigned int next_qtail = priv_->rx_dev.qtail + 1;  \
+   priv_->rx_dev.qtail = next_qtail % RX_DEVICE_BUFF_SIZE; })
+#define cnt_rxqbody(priv)  \
+   ({ typeof(priv) priv_ = (priv); \
+   unsigned int left_cnt = \
+   priv_->rx_dev.qtail + RX_DEVICE_BUFF_SIZE;  \
+   (left_cnt - (priv_->rx_dev.qhead)) % RX_DEVICE_BUFF_SIZE; })
 
 /* Read single byte from device address into byte (CMD52) */
 static int ks7010_sdio_readb(struct ks_wlan_private *priv, unsigned int 
address,
-- 
2.16.1

[PATCH 1/4] Staging: ks7010: sdio: Fix multiple use of arguments in RX/TX queue macros.

[Quytelda Kahja]

Use GCC extensions to prevent macro arguments from accidentally being evaluated
multiple times when the macro is called.

Signed-off-by: Quytelda Kahja 
---
 drivers/staging/ks7010/ks7010_sdio.c | 40 
 1 file changed, 27 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c 
b/drivers/staging/ks7010/ks7010_sdio.c
index 8cfdff198334..ffa7e2382353 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -32,19 +32,33 @@ static const struct sdio_device_id ks7010_sdio_ids[] = {
 };
 MODULE_DEVICE_TABLE(sdio, ks7010_sdio_ids);
 
-#define inc_txqhead(priv) \
-   (priv->tx_dev.qhead = (priv->tx_dev.qhead + 1) % TX_DEVICE_BUFF_SIZE)
-#define inc_txqtail(priv) \
-   (priv->tx_dev.qtail = (priv->tx_dev.qtail + 1) % TX_DEVICE_BUFF_SIZE)
-#define cnt_txqbody(priv) \
-   (((priv->tx_dev.qtail + TX_DEVICE_BUFF_SIZE) - (priv->tx_dev.qhead)) % 
TX_DEVICE_BUFF_SIZE)
-
-#define inc_rxqhead(priv) \
-   (priv->rx_dev.qhead = (priv->rx_dev.qhead + 1) % RX_DEVICE_BUFF_SIZE)
-#define inc_rxqtail(priv) \
-   (priv->rx_dev.qtail = (priv->rx_dev.qtail + 1) % RX_DEVICE_BUFF_SIZE)
-#define cnt_rxqbody(priv) \
-   (((priv->rx_dev.qtail + RX_DEVICE_BUFF_SIZE) - (priv->rx_dev.qhead)) % 
RX_DEVICE_BUFF_SIZE)
+#define inc_txqhead(priv)  \
+   ({ typeof(priv) priv_ = (priv); \
+   unsigned int next_qhead = priv_->tx_dev.qhead + 1;  \
+   priv_->tx_dev.qhead = next_qhead % TX_DEVICE_BUFF_SIZE; })
+#define inc_txqtail(priv)  \
+   ({ typeof(priv) priv_ = (priv); \
+   unsigned int next_qtail = priv_->tx_dev.qtail + 1;  \
+   priv_->tx_dev.qtail = next_qtail % TX_DEVICE_BUFF_SIZE; })
+#define cnt_txqbody(priv)  \
+   ({ typeof(priv) priv_ = (priv); \
+   unsigned int left_cnt = \
+   priv_->tx_dev.qtail + TX_DEVICE_BUFF_SIZE;  \
+   (left_cnt - (priv_->tx_dev.qhead)) % TX_DEVICE_BUFF_SIZE; })
+
+#define inc_rxqhead(priv)  \
+   ({ typeof(priv) priv_ = (priv); \
+   unsigned int next_qhead = priv_->rx_dev.qhead + 1;  \
+   priv_->rx_dev.qhead = next_qhead % RX_DEVICE_BUFF_SIZE; })
+#define inc_rxqtail(priv)  \
+   ({ typeof(priv) priv_ = (priv); \
+   unsigned int next_qtail = priv_->rx_dev.qtail + 1;  \
+   priv_->rx_dev.qtail = next_qtail % RX_DEVICE_BUFF_SIZE; })
+#define cnt_rxqbody(priv)  \
+   ({ typeof(priv) priv_ = (priv); \
+   unsigned int left_cnt = \
+   priv_->rx_dev.qtail + RX_DEVICE_BUFF_SIZE;  \
+   (left_cnt - (priv_->rx_dev.qhead)) % RX_DEVICE_BUFF_SIZE; })
 
 /* Read single byte from device address into byte (CMD52) */
 static int ks7010_sdio_readb(struct ks_wlan_private *priv, unsigned int 
address,
-- 
2.16.1