2014-06-04 17:45:53

by Remi Pommarel

Subject: [PATCH] Staging: rtl8821ae: use kmalloc instead of variable length stack arrays

This removes stack arrays of variable length and use kmalloc() instead, thus
removing the sparse warnings "Variable length array is used".

Signed-off-by: Remi Pommarel <[email protected]>
---
drivers/staging/rtl8821ae/efuse.c | 38 +++++++++++++++++++++++++++++++-------
1 file changed, 31 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/rtl8821ae/efuse.c b/drivers/staging/rtl8821ae/efuse.c
index 206012c..a41aa84 100644
--- a/drivers/staging/rtl8821ae/efuse.c
+++ b/drivers/staging/rtl8821ae/efuse.c
@@ -232,7 +232,7 @@ void read_efuse(struct ieee80211_hw *hw, u16 _offset, u16 _size_byte, u8 *pbuf)
{
struct rtl_priv *rtlpriv = rtl_priv(hw);
struct rtl_efuse *rtlefuse = rtl_efuse(rtl_priv(hw));
- u8 efuse_tbl[rtlpriv->cfg->maps[EFUSE_HWSET_MAX_SIZE]];
+ u8 *efuse_tbl;
u8 rtemp8[1];
u16 efuse_addr = 0;
u8 offset, wren;
@@ -243,7 +243,7 @@ void read_efuse(struct ieee80211_hw *hw, u16 _offset, u16 _size_byte, u8 *pbuf)
rtlpriv->cfg->maps[EFUSE_MAX_SECTION_MAP];
const u32 efuse_real_content_len =
rtlpriv->cfg->maps[EFUSE_REAL_CONTENT_SIZE];
- u16 efuse_word[efuse_max_section][EFUSE_MAX_WORD_UNIT];
+ u16 **efuse_word;
u16 efuse_utilized = 0;
u8 efuse_usage;

@@ -254,9 +254,26 @@ void read_efuse(struct ieee80211_hw *hw, u16 _offset, u16 _size_byte, u8 *pbuf)
return;
}

+ efuse_tbl = kmalloc(rtlpriv->cfg->maps[EFUSE_HWSET_MAX_SIZE] *
+ sizeof(*efuse_tbl), GFP_ATOMIC);
+ if (efuse_tbl == NULL)
+ return;
+
+ efuse_word = kcalloc(EFUSE_MAX_WORD_UNIT, sizeof(*efuse_word),
+ GFP_ATOMIC);
+ if (efuse_word == NULL)
+ goto out;
+
+ for (i = 0; i < EFUSE_MAX_WORD_UNIT; i++) {
+ efuse_word[i] = kmalloc(efuse_max_section *
+ sizeof(**efuse_word), GFP_ATOMIC);
+ if (efuse_word[i] == NULL)
+ goto done;
+ }
+
for (i = 0; i < efuse_max_section; i++)
for (j = 0; j < EFUSE_MAX_WORD_UNIT; j++)
- efuse_word[i][j] = 0xFFFF;
+ efuse_word[j][i] = 0xFFFF;

read_efuse_byte(hw, efuse_addr, rtemp8);
if (*rtemp8 != 0xFF) {
@@ -304,7 +321,7 @@ void read_efuse(struct ieee80211_hw *hw, u16 _offset, u16 _size_byte, u8 *pbuf)
read_efuse_byte(hw, efuse_addr, rtemp8);
efuse_addr++;
efuse_utilized++;
- efuse_word[offset][i] = (*rtemp8 &
+ efuse_word[i][offset] = (*rtemp8 &
0xff);

if (efuse_addr >=
@@ -318,7 +335,7 @@ void read_efuse(struct ieee80211_hw *hw, u16 _offset, u16 _size_byte, u8 *pbuf)
read_efuse_byte(hw, efuse_addr, rtemp8);
efuse_addr++;
efuse_utilized++;
- efuse_word[offset][i] |=
+ efuse_word[i][offset] |=
(((u16) * rtemp8 << 8) & 0xff00);

if (efuse_addr >= efuse_real_content_len)
@@ -341,9 +358,9 @@ void read_efuse(struct ieee80211_hw *hw, u16 _offset, u16 _size_byte, u8 *pbuf)
for (i = 0; i < efuse_max_section; i++) {
for (j = 0; j < EFUSE_MAX_WORD_UNIT; j++) {
efuse_tbl[(i * 8) + (j * 2)] =
- (efuse_word[i][j] & 0xff);
+ (efuse_word[j][i] & 0xff);
efuse_tbl[(i * 8) + ((j * 2) + 1)] =
- ((efuse_word[i][j] >> 8) & 0xff);
+ ((efuse_word[j][i] >> 8) & 0xff);
}
}

@@ -357,6 +374,13 @@ void read_efuse(struct ieee80211_hw *hw, u16 _offset, u16 _size_byte, u8 *pbuf)
(u8 *) & efuse_utilized);
rtlpriv->cfg->ops->set_hw_reg(hw, HW_VAR_EFUSE_USAGE,
(u8 *) & efuse_usage);
+
+done:
+ for (i = 0; i < EFUSE_MAX_WORD_UNIT; i++)
+ kfree(efuse_word[i]);
+ kfree(efuse_word);
+out:
+ kfree(efuse_tbl);
}

bool efuse_shadow_update_chk(struct ieee80211_hw *hw)
--
1.8.5.1

2014-06-10 08:27:17

by Dan Carpenter

Subject: Re: [PATCH] Staging: rtl8821ae: use kmalloc instead of variable length stack arrays

On Wed, Jun 04, 2014 at 07:45:45PM +0200, Remi Pommarel wrote:
> This removes stack arrays of variable length and use kmalloc() instead, thus
> removing the sparse warnings "Variable length array is used".
>
> Signed-off-by: Remi Pommarel <[email protected]>
> ---
> drivers/staging/rtl8821ae/efuse.c | 38 +++++++++++++++++++++++++++++++-------
> 1 file changed, 31 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/rtl8821ae/efuse.c b/drivers/staging/rtl8821ae/efuse.c
> index 206012c..a41aa84 100644
> --- a/drivers/staging/rtl8821ae/efuse.c
> +++ b/drivers/staging/rtl8821ae/efuse.c
> @@ -232,7 +232,7 @@ void read_efuse(struct ieee80211_hw *hw, u16 _offset, u16 _size_byte, u8 *pbuf)
> {
> struct rtl_priv *rtlpriv = rtl_priv(hw);
> struct rtl_efuse *rtlefuse = rtl_efuse(rtl_priv(hw));
> - u8 efuse_tbl[rtlpriv->cfg->maps[EFUSE_HWSET_MAX_SIZE]];
> + u8 *efuse_tbl;
> u8 rtemp8[1];
> u16 efuse_addr = 0;
> u8 offset, wren;
> @@ -243,7 +243,7 @@ void read_efuse(struct ieee80211_hw *hw, u16 _offset, u16 _size_byte, u8 *pbuf)
> rtlpriv->cfg->maps[EFUSE_MAX_SECTION_MAP];
> const u32 efuse_real_content_len =
> rtlpriv->cfg->maps[EFUSE_REAL_CONTENT_SIZE];
> - u16 efuse_word[efuse_max_section][EFUSE_MAX_WORD_UNIT];
> + u16 **efuse_word;
> u16 efuse_utilized = 0;
> u8 efuse_usage;
>
> @@ -254,9 +254,26 @@ void read_efuse(struct ieee80211_hw *hw, u16 _offset, u16 _size_byte, u8 *pbuf)
> return;
> }
>
> + efuse_tbl = kmalloc(rtlpriv->cfg->maps[EFUSE_HWSET_MAX_SIZE] *
> + sizeof(*efuse_tbl), GFP_ATOMIC);

sizeof(*efuse_tbl) is a complicated way to say 1. Just say:

efuse_tbl = kmalloc(rtlpriv->cfg->maps[EFUSE_HWSET_MAX_SIZE], GFP_ATOMIC);

Why is it GFP_ATOMIC btw? That wasn't immediately clear to me and I
didn't spend a lot of time looking.

> + if (efuse_tbl == NULL)
> + return;
> +
> + efuse_word = kcalloc(EFUSE_MAX_WORD_UNIT, sizeof(*efuse_word),
> + GFP_ATOMIC);
> + if (efuse_word == NULL)
> + goto out;
> +
> + for (i = 0; i < EFUSE_MAX_WORD_UNIT; i++) {
> + efuse_word[i] = kmalloc(efuse_max_section *
> + sizeof(**efuse_word), GFP_ATOMIC);
> + if (efuse_word[i] == NULL)
> + goto done;
> + }


Why is this so complicated? Why not just:

efuse_word = kmalloc(sizeof(*efuse_word) * efuse_max_section *
EFUSE_MAX_WORD_UNIT, GFP_ATOMIC);


> +
> for (i = 0; i < efuse_max_section; i++)
> for (j = 0; j < EFUSE_MAX_WORD_UNIT; j++)
> - efuse_word[i][j] = 0xFFFF;
> + efuse_word[j][i] = 0xFFFF;

Why are you re-ordering things here?

regards,
dan carpenter