2014-11-14 20:56:13

by Fabian Frédérick

[permalink] [raw]
Subject: [PATCH 1/1 net-next] wireless: remove unnecessary sizeof(u8)

sizeof(u8) is always 1.

Signed-off-by: Fabian Frederick <[email protected]>
---
drivers/net/wireless/b43/ppr.c | 2 +-
drivers/net/wireless/iwlwifi/pcie/trans.c | 2 +-
drivers/net/wireless/rtlwifi/efuse.c | 16 ++++++++--------
3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/b43/ppr.c b/drivers/net/wireless/b43/ppr.c
index 9a77027..6bc1c6f 100644
--- a/drivers/net/wireless/b43/ppr.c
+++ b/drivers/net/wireless/b43/ppr.c
@@ -28,7 +28,7 @@ void b43_ppr_clear(struct b43_wldev *dev, struct b43_ppr *ppr)
memset(ppr, 0, sizeof(*ppr));

/* Compile-time PPR check */
- BUILD_BUG_ON(sizeof(struct b43_ppr) != B43_PPR_RATES_NUM * sizeof(u8));
+ BUILD_BUG_ON(sizeof(struct b43_ppr) != B43_PPR_RATES_NUM);
}

void b43_ppr_add(struct b43_wldev *dev, struct b43_ppr *ppr, int diff)
diff --git a/drivers/net/wireless/iwlwifi/pcie/trans.c b/drivers/net/wireless/iwlwifi/pcie/trans.c
index 836725e..f016824 100644
--- a/drivers/net/wireless/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/iwlwifi/pcie/trans.c
@@ -1157,7 +1157,7 @@ static void iwl_trans_pcie_configure(struct iwl_trans *trans,
trans_pcie->n_no_reclaim_cmds = trans_cfg->n_no_reclaim_cmds;
if (trans_pcie->n_no_reclaim_cmds)
memcpy(trans_pcie->no_reclaim_cmds, trans_cfg->no_reclaim_cmds,
- trans_pcie->n_no_reclaim_cmds * sizeof(u8));
+ trans_pcie->n_no_reclaim_cmds);

trans_pcie->rx_buf_size_8k = trans_cfg->rx_buf_size_8k;
if (trans_pcie->rx_buf_size_8k)
diff --git a/drivers/net/wireless/rtlwifi/efuse.c b/drivers/net/wireless/rtlwifi/efuse.c
index 0b4082c..a3135c5 100644
--- a/drivers/net/wireless/rtlwifi/efuse.c
+++ b/drivers/net/wireless/rtlwifi/efuse.c
@@ -251,8 +251,8 @@ void read_efuse(struct ieee80211_hw *hw, u16 _offset, u16 _size_byte, u8 *pbuf)
}

/* allocate memory for efuse_tbl and efuse_word */
- efuse_tbl = kzalloc(rtlpriv->cfg->maps[EFUSE_HWSET_MAX_SIZE] *
- sizeof(u8), GFP_ATOMIC);
+ efuse_tbl = kzalloc(rtlpriv->cfg->maps[EFUSE_HWSET_MAX_SIZE],
+ GFP_ATOMIC);
if (!efuse_tbl)
return;
efuse_word = kzalloc(EFUSE_MAX_WORD_UNIT * sizeof(u16 *), GFP_ATOMIC);
@@ -733,8 +733,8 @@ static int efuse_pg_packet_read(struct ieee80211_hw *hw, u8 offset, u8 *data)
if (offset > 15)
return false;

- memset(data, 0xff, PGPKT_DATA_SIZE * sizeof(u8));
- memset(tmpdata, 0xff, PGPKT_DATA_SIZE * sizeof(u8));
+ memset(data, 0xff, PGPKT_DATA_SIZE);
+ memset(tmpdata, 0xff, PGPKT_DATA_SIZE);

while (continual && (efuse_addr < EFUSE_MAX_SIZE)) {
if (readstate & PG_STATE_HEADER) {
@@ -772,7 +772,7 @@ static void efuse_write_data_case1(struct ieee80211_hw *hw, u16 *efuse_addr,
struct rtl_priv *rtlpriv = rtl_priv(hw);
struct pgpkt_struct tmp_pkt;
int dataempty = true;
- u8 originaldata[8 * sizeof(u8)];
+ u8 originaldata[8];
u8 badworden = 0x0F;
u8 match_word_en, tmp_word_en;
u8 tmpindex;
@@ -881,7 +881,7 @@ static void efuse_write_data_case2(struct ieee80211_hw *hw, u16 *efuse_addr,
struct pgpkt_struct tmp_pkt;
u8 pg_header;
u8 tmp_header;
- u8 originaldata[8 * sizeof(u8)];
+ u8 originaldata[8];
u8 tmp_word_cnts;
u8 badworden = 0x0F;

@@ -904,7 +904,7 @@ static void efuse_write_data_case2(struct ieee80211_hw *hw, u16 *efuse_addr,

tmp_word_cnts = efuse_calculate_word_cnts(tmp_pkt.word_en);

- memset(originaldata, 0xff, 8 * sizeof(u8));
+ memset(originaldata, 0xff, 8);

if (efuse_pg_packet_read(hw, tmp_pkt.offset, originaldata)) {
badworden = enable_efuse_data_write(hw,
@@ -962,7 +962,7 @@ static int efuse_pg_packet_write(struct ieee80211_hw *hw,
target_pkt.offset = offset;
target_pkt.word_en = word_en;

- memset(target_pkt.data, 0xFF, 8 * sizeof(u8));
+ memset(target_pkt.data, 0xFF, 8);

efuse_word_enable_data_read(word_en, data, target_pkt.data);
target_word_cnts = efuse_calculate_word_cnts(target_pkt.word_en);
--
1.9.3


2014-11-16 22:33:59

by Julian Calaby

[permalink] [raw]
Subject: Re: [PATCH 1/1 net-next] wireless: remove unnecessary sizeof(u8)

Hi Fabian,

On Sat, Nov 15, 2014 at 7:55 AM, Fabian Frederick <[email protected]> wrote:
> sizeof(u8) is always 1.

I thought that sizeof(*variable) was preferred over sizeof(type), so
shouldn't these be switched to that format instead?

(I know that this is all no-op, but it should reduce the potential for
highly unlikely bugs in the future. Also, the extra processing is
compile-time not run-time.)

Thanks,

Julian Calaby


> Signed-off-by: Fabian Frederick <[email protected]>
> ---
> drivers/net/wireless/b43/ppr.c | 2 +-
> drivers/net/wireless/iwlwifi/pcie/trans.c | 2 +-
> drivers/net/wireless/rtlwifi/efuse.c | 16 ++++++++--------
> 3 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/wireless/b43/ppr.c b/drivers/net/wireless/b43/ppr.c
> index 9a77027..6bc1c6f 100644
> --- a/drivers/net/wireless/b43/ppr.c
> +++ b/drivers/net/wireless/b43/ppr.c
> @@ -28,7 +28,7 @@ void b43_ppr_clear(struct b43_wldev *dev, struct b43_ppr *ppr)
> memset(ppr, 0, sizeof(*ppr));
>
> /* Compile-time PPR check */
> - BUILD_BUG_ON(sizeof(struct b43_ppr) != B43_PPR_RATES_NUM * sizeof(u8));
> + BUILD_BUG_ON(sizeof(struct b43_ppr) != B43_PPR_RATES_NUM);
> }
>
> void b43_ppr_add(struct b43_wldev *dev, struct b43_ppr *ppr, int diff)
> diff --git a/drivers/net/wireless/iwlwifi/pcie/trans.c b/drivers/net/wireless/iwlwifi/pcie/trans.c
> index 836725e..f016824 100644
> --- a/drivers/net/wireless/iwlwifi/pcie/trans.c
> +++ b/drivers/net/wireless/iwlwifi/pcie/trans.c
> @@ -1157,7 +1157,7 @@ static void iwl_trans_pcie_configure(struct iwl_trans *trans,
> trans_pcie->n_no_reclaim_cmds = trans_cfg->n_no_reclaim_cmds;
> if (trans_pcie->n_no_reclaim_cmds)
> memcpy(trans_pcie->no_reclaim_cmds, trans_cfg->no_reclaim_cmds,
> - trans_pcie->n_no_reclaim_cmds * sizeof(u8));
> + trans_pcie->n_no_reclaim_cmds);
>
> trans_pcie->rx_buf_size_8k = trans_cfg->rx_buf_size_8k;
> if (trans_pcie->rx_buf_size_8k)
> diff --git a/drivers/net/wireless/rtlwifi/efuse.c b/drivers/net/wireless/rtlwifi/efuse.c
> index 0b4082c..a3135c5 100644
> --- a/drivers/net/wireless/rtlwifi/efuse.c
> +++ b/drivers/net/wireless/rtlwifi/efuse.c
> @@ -251,8 +251,8 @@ void read_efuse(struct ieee80211_hw *hw, u16 _offset, u16 _size_byte, u8 *pbuf)
> }
>
> /* allocate memory for efuse_tbl and efuse_word */
> - efuse_tbl = kzalloc(rtlpriv->cfg->maps[EFUSE_HWSET_MAX_SIZE] *
> - sizeof(u8), GFP_ATOMIC);
> + efuse_tbl = kzalloc(rtlpriv->cfg->maps[EFUSE_HWSET_MAX_SIZE],
> + GFP_ATOMIC);
> if (!efuse_tbl)
> return;
> efuse_word = kzalloc(EFUSE_MAX_WORD_UNIT * sizeof(u16 *), GFP_ATOMIC);
> @@ -733,8 +733,8 @@ static int efuse_pg_packet_read(struct ieee80211_hw *hw, u8 offset, u8 *data)
> if (offset > 15)
> return false;
>
> - memset(data, 0xff, PGPKT_DATA_SIZE * sizeof(u8));
> - memset(tmpdata, 0xff, PGPKT_DATA_SIZE * sizeof(u8));
> + memset(data, 0xff, PGPKT_DATA_SIZE);
> + memset(tmpdata, 0xff, PGPKT_DATA_SIZE);
>
> while (continual && (efuse_addr < EFUSE_MAX_SIZE)) {
> if (readstate & PG_STATE_HEADER) {
> @@ -772,7 +772,7 @@ static void efuse_write_data_case1(struct ieee80211_hw *hw, u16 *efuse_addr,
> struct rtl_priv *rtlpriv = rtl_priv(hw);
> struct pgpkt_struct tmp_pkt;
> int dataempty = true;
> - u8 originaldata[8 * sizeof(u8)];
> + u8 originaldata[8];
> u8 badworden = 0x0F;
> u8 match_word_en, tmp_word_en;
> u8 tmpindex;
> @@ -881,7 +881,7 @@ static void efuse_write_data_case2(struct ieee80211_hw *hw, u16 *efuse_addr,
> struct pgpkt_struct tmp_pkt;
> u8 pg_header;
> u8 tmp_header;
> - u8 originaldata[8 * sizeof(u8)];
> + u8 originaldata[8];
> u8 tmp_word_cnts;
> u8 badworden = 0x0F;
>
> @@ -904,7 +904,7 @@ static void efuse_write_data_case2(struct ieee80211_hw *hw, u16 *efuse_addr,
>
> tmp_word_cnts = efuse_calculate_word_cnts(tmp_pkt.word_en);
>
> - memset(originaldata, 0xff, 8 * sizeof(u8));
> + memset(originaldata, 0xff, 8);
>
> if (efuse_pg_packet_read(hw, tmp_pkt.offset, originaldata)) {
> badworden = enable_efuse_data_write(hw,
> @@ -962,7 +962,7 @@ static int efuse_pg_packet_write(struct ieee80211_hw *hw,
> target_pkt.offset = offset;
> target_pkt.word_en = word_en;
>
> - memset(target_pkt.data, 0xFF, 8 * sizeof(u8));
> + memset(target_pkt.data, 0xFF, 8);
>
> efuse_word_enable_data_read(word_en, data, target_pkt.data);
> target_word_cnts = efuse_calculate_word_cnts(target_pkt.word_en);
> --
> 1.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
Julian Calaby

Email: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/

2014-11-17 05:56:30

by Fabian Frédérick

[permalink] [raw]
Subject: Re: [PATCH 1/1 net-next] wireless: remove unnecessary sizeof(u8)



> On 16 November 2014 at 23:33 Julian Calaby <[email protected]> wrote:
>
>
> Hi Fabian,
>
> On Sat, Nov 15, 2014 at 7:55 AM, Fabian Frederick <[email protected]> wrote:
> > sizeof(u8) is always 1.
>
> I thought that sizeof(*variable) was preferred over sizeof(type), so
> shouldn't these be switched to that format instead?
>
> (I know that this is all no-op, but it should reduce the potential for
> highly unlikely bugs in the future. Also, the extra processing is
> compile-time not run-time.)
>
> Thanks,

Hi Julian,

Of course but char/u8/s8... allocations never use it and result would be the
same:
factor 1 multiplication.

Those rare occurrences (+- 30 in the whole kernel) where we have
sizeof(u8/s8) is ambiguous.

Having a patch removing it gives a pointer...
If the developer meant something else, he will be able to fix it.

Regards,
Fabian

2014-11-17 13:46:00

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH 1/1 net-next] wireless: remove unnecessary sizeof(u8)

On Mon, Nov 17, 2014 at 06:56:27AM +0100, Fabian Frederick wrote:
>
>
> > On 16 November 2014 at 23:33 Julian Calaby <[email protected]> wrote:
> >
> >
> > Hi Fabian,
> >
> > On Sat, Nov 15, 2014 at 7:55 AM, Fabian Frederick <[email protected]> wrote:
> > > sizeof(u8) is always 1.
> >
> > I thought that sizeof(*variable) was preferred over sizeof(type), so
> > shouldn't these be switched to that format instead?
> >
> > (I know that this is all no-op, but it should reduce the potential for
> > highly unlikely bugs in the future. Also, the extra processing is
> > compile-time not run-time.)
> >
> > Thanks,
>
> Hi Julian,
>
> Of course but char/u8/s8... allocations never use it and result would be the
> same:
> factor 1 multiplication.
>
> Those rare occurrences (+- 30 in the whole kernel) where we have
> sizeof(u8/s8) is ambiguous.
>
> Having a patch removing it gives a pointer...
> If the developer meant something else, he will be able to fix it.
>
> Regards,
> Fabian

sizeof(*variable) still seems safer. Are the compilers unable to
optimize-away a "multiply by one"?

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2014-11-18 16:12:30

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH 1/1 net-next] wireless: remove unnecessary sizeof(u8)

On 11/18/2014 07:34 AM, Gheorhios wrote:
> Anyone could gently send the link for downloading B43 linux drivers for
> this procedure?
>
>
>
> Then transfer it over to your Ubuntu box.
>
> Now in your Ubuntu Box [computer] please make your way to your Home folder.
>
> Once you are at your home folder right click on your home folder and make a
> new folder and call it wireless.
>
> Now that you have made a new folder called wireless in your home directory,
> it is time to move the downloaded file into the new folder called wireless.
>
> Move The Wireless Folder To The Firmware Directory
>
> sudo cp -r ~/wireless/* /lib/firmware/
>
> Now let's double check to make sure the download made it to the firmware
> directory. To do this type this into the terminal:
>
> ls /lib/firmware
>
> Ok so now that the download is in the firmware directory we need to go to
> that directory. To go there open your terminal and type in:
>
> cd /lib/firmware
>
> Now that you have changed directories let's double check to make sure you
> are in the right directory, this next code tells us where we are in the
> computer file directory. This next code stands for "print working
> directory".
>
> pwd
>
> Are you at /lib/firmware if so good if not go back one step.
>
> Now that we are in the firmware directory. We have to extract the download,
> to do this type in:
>
> sudo -s
>
> Then enter your password then:
>
> tar xvf b43-all-fw.tar_.gz
>
> Now is the firmware extracted properly? check by typing:
>
> ls /lib/firmware/b43
>
> or:
>
> ls /lib/firmware/b43legacy
>
> Do you see the ucode files? if so then delete the gz file:
>
> sudo rm *.gz
>
> Then:
>
> exit
>
> Reboot

No, I do not know where that file is found. Even if I knew of such a file,
Broadcom has expressly declined to provide that firmware for redistribution.
Posting such a file could invite legal action. If someone else has violated
Broadcom's directive, I would not facilitate that violation.

Rather than doing that, the link at
http://wireless.kernel.org/en/users/Drivers/b43#firmwareinstallation shows what
to do for Ubuntu installations. Follow those instructions - they refer to a
legal way to get the firmware. I would have thought that asking this question on
an Ubuntu Forum would have been more productive.

By the way, piggybacking your request on this thread is very bad netiquette. You
should not have done a "reply-to".

Larry