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 <r...@triplefau.lt>
> ---
>  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

_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to