2016-01-02 14:40:52

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 0/3] net-rsi: Fine-tuning for two function implementations

From: Markus Elfring <[email protected]>
Date: Sat, 2 Jan 2016 15:36:25 +0100

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (3):
Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()
Delete unnecessary variable initialisations in rsi_send_data_pkt()
Replace variable initialisations by assignments in rsi_send_data_pkt()

drivers/net/wireless/rsi/rsi_91x_pkt.c | 28 +++++++++++++++-------------
1 file changed, 15 insertions(+), 13 deletions(-)

--
2.6.3



2016-01-04 10:44:28

by SF Markus Elfring

[permalink] [raw]
Subject: Re: [PATCH 1/3] rsi: Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()

> These patches are labour intensive to review because you can't just do
> it in the email client.

Thanks for your general interest.


> Also you were not able to review it properly yourself and introduced
> a bug.

I admit that it can happen during my software development that I overlook
implementation details somehow.


> I am often remove initializers but it's normally because I am changing
> something else which makes it worthwhile.

It is nice to hear that you are also occasionally looking for similar
update candidates.


> This patch is the correct thing but it's not "worthwhile".

I find this view interesting.


> Please stop sending cleanup patches, Markus. Just send fixes.

How often will source code clean-up fix something?


May I resend a consistent patch series for the source file
"drivers/net/wireless/rsi/rsi_91x_pkt.c" in the near future?

Regards,
Markus

2016-01-04 12:33:29

by SF Markus Elfring

[permalink] [raw]
Subject: Re: rsi: Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()

>> May I resend a consistent patch series for the source file
>> "drivers/net/wireless/rsi/rsi_91x_pkt.c" in the near future?
>
> If you were sending checkpatch.pl fixes that would be easier to deal with

Does this feedback mean that you would accept any more suggestions around
source code updates which are derived from recommendations of this script?


> but you are sending hundreds of "controversial" cleanups.

It depends on the time range you look at for my proposals.


> They are controversial in the sense that they don't fix anything
> against official kernel style

I find that I suggested also few changes that fit to this aspect.


> and they go against the author's original intention.

Can it occasionally help to reconsider the "first approach"?


> I tend to agree that useless initializers are bad

Would any more software developers like to share their opinions on this detail?


> and disable GCCs uninitialized variable warnings

I hope that this software area can be also improved.


> but just because I agree with you doesn't make it official kernel style.

That is fine. - Will it become useful to clarify any extensions
to a document like "CodingStyle"?


> It's slightly rude to go against the author's intention.

I just dare to propose further special changes.

Regards,
Markus


2016-01-19 12:40:40

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] rsi: Replace variable initialisations by assignments in rsi_send_data_pkt()

Still makes no sense for adapter like Francois Romieu said.

regards,
dan carpenter


2016-01-04 14:25:32

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 1/3] rsi: Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()

On Mon, Jan 04, 2016 at 02:17:40PM +0100, Bj?rn Mork wrote:
> Dan Carpenter <[email protected]> writes:
>
> > Please stop sending cleanup patches, Markus. Just send fixes.
>
> Thanks for your continued but unwarranted belief in AI.
>

I always tell people that I am very mechanical and you can rely on me to
send predictable responses...

> Do you mind if I remind you of https://lkml.org/lkml/2014/11/3/162 ?
> I am sure there are lots and lots of other examples. There is no reason
> to believe this will ever stop. He just goes into Eliza mode.

Yup.

I feel some sense of responsibility for any patches where kernel-janitors
is on the CC but I'm having a hard time dealing with all of Markus's
patches. Normally you just respond to the first patch and people change
the later patches but, as you put it, Markus just goes into ELIZA mode.

regards,
dan carpenter


2016-01-02 15:12:36

by SF Markus Elfring

[permalink] [raw]
Subject: net-rsi: Reconsider usage of variable "vap_id" in rsi_send_mgmt_pkt()

Hello,

I have taken another look at the implementation of the function "rsi_send_mgmt_pkt".
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/drivers/net/wireless/rsi/rsi_91x_pkt.c?id=e8c58e7a5a106c3d557fccd01cd4d1128f9bab38#n114

I find the following statement combination interesting there.


u8 vap_id = 0;

msg[7] |= cpu_to_le16(vap_id << 8);


I would appreciate a further clarification.
Does a shift operation for a variable which contains zero indicate an open issue?

Regards,
Markus

2016-01-04 09:38:36

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 1/3] rsi: Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()

Btw, GCC misses a lot of uninitialized variable bugs. I have a Smatch
check which sometimes catches the bugs that GCC misses but you should
not rely on the tools here. These patches need to be reviewed manually.

And the "goto err" before the initialization makes everything more
complicated (that's actually what caused the bug in this patch, in fact).

regards,
dan carpenter


2016-01-15 13:05:00

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH v3 0/3] net-rsi: Fine-tuning for two function implementations

From: Markus Elfring <[email protected]>
Date: Fri, 15 Jan 2016 13:54:43 +0100

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (3):
Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()
Delete unnecessary variable initialisations in rsi_send_data_pkt()
Replace variable initialisations by assignments in rsi_send_data_pkt()

drivers/net/wireless/rsi/rsi_91x_pkt.c | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)

---

v3: Rebase proposed changes on the source files for the software
"Linux next-20160114".

v2: Unfortunately, the first update step from this series contained
an inappropriate suggestion.
Thus fix that.

--
2.6.3


2016-01-05 09:48:03

by Julian Calaby

[permalink] [raw]
Subject: Re: rsi: Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()

Hi Markus,

On Tue, Jan 5, 2016 at 7:29 PM, SF Markus Elfring
<[email protected]> wrote:
>> That said, if you figure out some change that produces significant
>> reductions in code or binary size on multiple architectures without
>> making things more complicated, less readable or making the code or
>> binary size larger, then by all means propose it.
>
> Are you looking also for "a proof" that such changes are worthwhile?

It'd be better than "I think doing things this way is better", which
is the hallmark of most of your patch sets. (Admittedly not this one,
but this one is where the discussion is now, so that's where we're
discussing it.)

>> "This makes things smaller" carries much more weight than
>> "I think this is better".
>
> Can the discussed implementation of a function like "rsi_send_mgmt_pkt"
> become a bit smaller by the deletion of extra variable initialisations

I'm talking in general.

In this case you're asking people to review a patch which requires a
lot of careful review for a fairly minor improvement. I must also note
that you haven't CC'd the people who wrote this driver, so it's
possible that the only people who have reviewed it aren't experts in
the code.

The patches you sent recently which moved labels into if statements
were a clear case of "I think this is better" where any actual benefit
from the changes was eclipsed by the style and readability issues they
introduced.

>> Almost all of the changes you've proposed that have seen any
>> discussion whatsoever fall into the latter category.
>
> Thanks for your interesting feedback.

No problem.

> Can a further constructive dialogue evolve from the presented information?

Part of the issue here is that you don't seem to be listening to the
discussion of your patches, or if you are, you're not significantly
changing your approach or attitude in response.

Every time you send a set of patches, there are legitimate issues
which people raise, and every time they are discussed, you assert that
your patches improve things and seem to ignore the concerns people
raise.

I've seen this same pattern of discussion here with these patches,
with your patches to move labels into if statements, with the patches
you sent late June last year, your patches to remove conditions before
kfree() and friends, etc.

You need to change you attitude: just because you can see some benefit
from your patches doesn't mean others do and it doesn't mean that
they're willing to accept them.

Thanks,

--
Julian Calaby

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

2016-01-02 15:07:58

by Francois Romieu

[permalink] [raw]
Subject: Re: [PATCH 3/3] rsi: Replace variable initialisations by assignments in rsi_send_data_pkt()

SF Markus Elfring <[email protected]> :
> From: Markus Elfring <[email protected]>
> Date: Sat, 2 Jan 2016 15:25:34 +0100
>
> Replace explicit initialisation for two local variables at the beginning
> by assignments.

It makes no sense for the 'adapter' variable.

--
Ueimor

2016-01-15 13:10:45

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH v3 2/3] rsi: Delete unnecessary variable initialisations in rsi_send_data_pkt()

>From 017d1bb49f46266ffeb33178ddd3022d6b341d71 Mon Sep 17 00:00:00 2001
From: Markus Elfring <[email protected]>
Date: Fri, 15 Jan 2016 13:35:47 +0100
Subject: [PATCH 2/3] rsi: Delete unnecessary variable initialisations in
rsi_send_data_pkt()

Omit explicit initialisation at the beginning for four local variables
which are redefined before their first use.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/net/wireless/rsi/rsi_91x_pkt.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_pkt.c b/drivers/net/wireless/rsi/rsi_91x_pkt.c
index 571eaba..4322df1 100644
--- a/drivers/net/wireless/rsi/rsi_91x_pkt.c
+++ b/drivers/net/wireless/rsi/rsi_91x_pkt.c
@@ -27,15 +27,15 @@
int rsi_send_data_pkt(struct rsi_common *common, struct sk_buff *skb)
{
struct rsi_hw *adapter = common->priv;
- struct ieee80211_hdr *tmp_hdr = NULL;
+ struct ieee80211_hdr *tmp_hdr;
struct ieee80211_tx_info *info;
struct skb_info *tx_params;
- struct ieee80211_bss_conf *bss = NULL;
+ struct ieee80211_bss_conf *bss;
int status = -EINVAL;
u8 ieee80211_size = MIN_802_11_HDR_LEN;
- u8 extnd_size = 0;
+ u8 extnd_size;
__le16 *frame_desc;
- u16 seq_num = 0;
+ u16 seq_num;

info = IEEE80211_SKB_CB(skb);
bss = &info->control.vif->bss_conf;
--
2.6.3


2016-01-04 13:18:13

by Bjørn Mork

[permalink] [raw]
Subject: Re: [PATCH 1/3] rsi: Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()

Dan Carpenter <[email protected]> writes:

> Please stop sending cleanup patches, Markus. Just send fixes.

Thanks for your continued but unwarranted belief in AI.

Do you mind if I remind you of https://lkml.org/lkml/2014/11/3/162 ?
I am sure there are lots and lots of other examples. There is no reason
to believe this will ever stop. He just goes into Eliza mode.


Bjørn

2016-01-15 13:09:28

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH v3 1/3] rsi: Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()

From: Markus Elfring <[email protected]>
Date: Fri, 15 Jan 2016 13:30:39 +0100

Omit explicit initialisation at the beginning for four local variables
which are redefined before their first use.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/net/wireless/rsi/rsi_91x_pkt.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_pkt.c b/drivers/net/wireless/rsi/rsi_91x_pkt.c
index 702593f..571eaba 100644
--- a/drivers/net/wireless/rsi/rsi_91x_pkt.c
+++ b/drivers/net/wireless/rsi/rsi_91x_pkt.c
@@ -123,15 +123,15 @@ int rsi_send_mgmt_pkt(struct rsi_common *common,
struct sk_buff *skb)
{
struct rsi_hw *adapter = common->priv;
- struct ieee80211_hdr *wh = NULL;
+ struct ieee80211_hdr *wh;
struct ieee80211_tx_info *info;
- struct ieee80211_bss_conf *bss = NULL;
+ struct ieee80211_bss_conf *bss;
struct ieee80211_hw *hw = adapter->hw;
struct ieee80211_conf *conf = &hw->conf;
struct skb_info *tx_params;
int status = -E2BIG;
- __le16 *msg = NULL;
- u8 extnd_size = 0;
+ __le16 *msg;
+ u8 extnd_size;
u8 vap_id = 0;

info = IEEE80211_SKB_CB(skb);
--
2.6.3


2016-01-04 23:54:40

by Julian Calaby

[permalink] [raw]
Subject: Re: rsi: Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()

Hi Markus,

On Mon, Jan 4, 2016 at 11:33 PM, SF Markus Elfring
<[email protected]> wrote:
>>> May I resend a consistent patch series for the source file
>>> "drivers/net/wireless/rsi/rsi_91x_pkt.c" in the near future?
>>
>> If you were sending checkpatch.pl fixes that would be easier to deal with
>
> Does this feedback mean that you would accept any more suggestions around
> source code updates which are derived from recommendations of this script?

A good rule of thumb here would be that if people start complaining
about a particular type of change, stop sending them.

Another good rule of thumb is to try to "rock the boat" on coding
style and conventions as little as possible. Just because it's
possible doesn't mean that people want to do it.

That said, if you figure out some change that produces significant
reductions in code or binary size on multiple architectures without
making things more complicated, less readable or making the code or
binary size larger, then by all means propose it. "This makes things
smaller" carries much more weight than "I think this is better".
Almost all of the changes you've proposed that have seen any
discussion whatsoever fall into the latter category.

Thanks,

--
Julian Calaby

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

2016-01-05 16:23:18

by SF Markus Elfring

[permalink] [raw]
Subject: Re: rsi: Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()

> Every time you send a set of patches,

I suggested some updates for Linux source files since October 2014.


> there are legitimate issues which people raise,

There was usual feedback.


> and every time they are discussed,

The discussion results were mixed between acceptance
and usual disagreement.


> you assert that your patches improve things

I guess that should be the default intention of every patch, shouldn't it?


> and seem to ignore the concerns people raise.

I hope not. - But I can imagine that you might understand some responses
from contributors in this way.
Are you waiting for another clarification on a specific issue?


> I've seen this same pattern of discussion here with these patches,
> with your patches to move labels into if statements, with the patches
> you sent late June last year, your patches to remove conditions before
> kfree() and friends, etc.

It seems that communication difficulties come partly from the fact
that I chose search patterns from static source code analysis so far
which belong to an error category that gets a lower priority.


> You need to change you attitude: just because you can see some benefit
> from your patches doesn't mean others do and it doesn't mean that
> they're willing to accept them.

I understand your advice.

Further update suggestions with higher importance might follow for various
software areas in the future.

Regards,
Markus

2016-01-02 14:45:22

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 3/3] rsi: Replace variable initialisations by assignments in rsi_send_data_pkt()

From: Markus Elfring <[email protected]>
Date: Sat, 2 Jan 2016 15:25:34 +0100

Replace explicit initialisation for two local variables at the beginning
by assignments.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/net/wireless/rsi/rsi_91x_pkt.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_pkt.c b/drivers/net/wireless/rsi/rsi_91x_pkt.c
index ec65e1c..fe36e7d 100644
--- a/drivers/net/wireless/rsi/rsi_91x_pkt.c
+++ b/drivers/net/wireless/rsi/rsi_91x_pkt.c
@@ -26,12 +26,12 @@
*/
int rsi_send_data_pkt(struct rsi_common *common, struct sk_buff *skb)
{
- struct rsi_hw *adapter = common->priv;
+ struct rsi_hw *adapter;
struct ieee80211_hdr *tmp_hdr;
struct ieee80211_tx_info *info;
struct skb_info *tx_params;
struct ieee80211_bss_conf *bss;
- int status = -EINVAL;
+ int status;
u8 ieee80211_size = MIN_802_11_HDR_LEN;
u8 extnd_size;
__le16 *frame_desc;
@@ -41,8 +41,10 @@ int rsi_send_data_pkt(struct rsi_common *common, struct sk_buff *skb)
bss = &info->control.vif->bss_conf;
tx_params = (struct skb_info *)info->driver_data;

- if (!bss->assoc)
+ if (!bss->assoc) {
+ status = -EINVAL;
goto err;
+ }

tmp_hdr = (struct ieee80211_hdr *)&skb->data[0];
seq_num = (le16_to_cpu(tmp_hdr->seq_ctrl) >> 4);
@@ -97,7 +99,7 @@ int rsi_send_data_pkt(struct rsi_common *common, struct sk_buff *skb)
frame_desc[7] = cpu_to_le16(((tx_params->tid & 0xf) << 4) |
(skb->priority & 0xf) |
(tx_params->sta_id << 8));
-
+ adapter = common->priv;
status = adapter->host_intf_write_pkt(common->priv,
skb->data,
skb->len);
--
2.6.3


2016-01-02 14:44:30

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 2/3] rsi: Delete unnecessary variable initialisations in rsi_send_data_pkt()

From: Markus Elfring <[email protected]>
Date: Sat, 2 Jan 2016 15:15:12 +0100

Omit explicit initialisation at the beginning for four local variables
which are redefined before their first use.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/net/wireless/rsi/rsi_91x_pkt.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_pkt.c b/drivers/net/wireless/rsi/rsi_91x_pkt.c
index ee98f5b..ec65e1c 100644
--- a/drivers/net/wireless/rsi/rsi_91x_pkt.c
+++ b/drivers/net/wireless/rsi/rsi_91x_pkt.c
@@ -27,15 +27,15 @@
int rsi_send_data_pkt(struct rsi_common *common, struct sk_buff *skb)
{
struct rsi_hw *adapter = common->priv;
- struct ieee80211_hdr *tmp_hdr = NULL;
+ struct ieee80211_hdr *tmp_hdr;
struct ieee80211_tx_info *info;
struct skb_info *tx_params;
- struct ieee80211_bss_conf *bss = NULL;
+ struct ieee80211_bss_conf *bss;
int status = -EINVAL;
u8 ieee80211_size = MIN_802_11_HDR_LEN;
- u8 extnd_size = 0;
+ u8 extnd_size;
__le16 *frame_desc;
- u16 seq_num = 0;
+ u16 seq_num;

info = IEEE80211_SKB_CB(skb);
bss = &info->control.vif->bss_conf;
--
2.6.3


2016-01-02 18:27:46

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH v2 1/3] rsi: Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()

From: Markus Elfring <[email protected]>
Date: Sat, 2 Jan 2016 19:22:36 +0100

Omit explicit initialisation at the beginning for four local variables
which are redefined before their first use.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/net/wireless/rsi/rsi_91x_pkt.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_pkt.c b/drivers/net/wireless/rsi/rsi_91x_pkt.c
index 702593f..571eaba 100644
--- a/drivers/net/wireless/rsi/rsi_91x_pkt.c
+++ b/drivers/net/wireless/rsi/rsi_91x_pkt.c
@@ -123,15 +123,15 @@ int rsi_send_mgmt_pkt(struct rsi_common *common,
struct sk_buff *skb)
{
struct rsi_hw *adapter = common->priv;
- struct ieee80211_hdr *wh = NULL;
+ struct ieee80211_hdr *wh;
struct ieee80211_tx_info *info;
- struct ieee80211_bss_conf *bss = NULL;
+ struct ieee80211_bss_conf *bss;
struct ieee80211_hw *hw = adapter->hw;
struct ieee80211_conf *conf = &hw->conf;
struct skb_info *tx_params;
int status = -E2BIG;
- __le16 *msg = NULL;
- u8 extnd_size = 0;
+ __le16 *msg;
+ u8 extnd_size;
u8 vap_id = 0;

info = IEEE80211_SKB_CB(skb);
--
2.6.3


2016-01-05 08:30:17

by SF Markus Elfring

[permalink] [raw]
Subject: Re: rsi: Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()

> That said, if you figure out some change that produces significant
> reductions in code or binary size on multiple architectures without
> making things more complicated, less readable or making the code or
> binary size larger, then by all means propose it.

Are you looking also for "a proof" that such changes are worthwhile?


> "This makes things smaller" carries much more weight than
> "I think this is better".

Can the discussed implementation of a function like "rsi_send_mgmt_pkt"
become a bit smaller by the deletion of extra variable initialisations


> Almost all of the changes you've proposed that have seen any
> discussion whatsoever fall into the latter category.

Thanks for your interesting feedback.

Can a further constructive dialogue evolve from the presented information?

Regards,
Markus

2016-01-04 09:29:07

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 1/3] rsi: Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()

These patches are labour intensive to review because you can't just do
it in the email client. Also you were not able to review it properly
yourself and introduced a bug.

I am often remove initializers but it's normally because I am changing
something else which makes it worthwhile. This patch is the correct
thing but it's not "worthwhile". It is not a good use of my time.

Please stop sending cleanup patches, Markus. Just send fixes.

regards,
dan carpenter


2016-01-04 11:48:56

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 1/3] rsi: Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()

On Mon, Jan 04, 2016 at 11:44:15AM +0100, SF Markus Elfring wrote:
> > Please stop sending cleanup patches, Markus. Just send fixes.
>
> How often will source code clean-up fix something?
>
>
> May I resend a consistent patch series for the source file
> "drivers/net/wireless/rsi/rsi_91x_pkt.c" in the near future?

If you were sending checkpatch.pl fixes that would be easier to deal
with but you are sending hundreds of "controversial" cleanups. They are
controversial in the sense that they don't fix anything against official
kernel style and they go against the author's original intention. I
tend to agree that useless initializers are bad and disable GCCs
uninitialized variable warnings but just because I agree with you
doesn't make it official kernel style. It's slightly rude to go against
the author's intention.

regards,
dan carpenter

2016-01-04 17:14:16

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 1/3] rsi: Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()

From: Dan Carpenter <[email protected]>
Date: Mon, 4 Jan 2016 12:28:57 +0300

> Please stop sending cleanup patches, Markus. Just send fixes.

+1

2016-01-02 14:43:22

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 1/3] rsi: Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()

From: Markus Elfring <[email protected]>
Date: Sat, 2 Jan 2016 14:54:30 +0100

Omit explicit initialisation at the beginning for five local variables
which are redefined before their first use.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/net/wireless/rsi/rsi_91x_pkt.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_pkt.c b/drivers/net/wireless/rsi/rsi_91x_pkt.c
index 702593f..ee98f5b 100644
--- a/drivers/net/wireless/rsi/rsi_91x_pkt.c
+++ b/drivers/net/wireless/rsi/rsi_91x_pkt.c
@@ -123,15 +123,15 @@ int rsi_send_mgmt_pkt(struct rsi_common *common,
struct sk_buff *skb)
{
struct rsi_hw *adapter = common->priv;
- struct ieee80211_hdr *wh = NULL;
+ struct ieee80211_hdr *wh;
struct ieee80211_tx_info *info;
- struct ieee80211_bss_conf *bss = NULL;
+ struct ieee80211_bss_conf *bss;
struct ieee80211_hw *hw = adapter->hw;
struct ieee80211_conf *conf = &hw->conf;
struct skb_info *tx_params;
- int status = -E2BIG;
- __le16 *msg = NULL;
- u8 extnd_size = 0;
+ int status;
+ __le16 *msg;
+ u8 extnd_size;
u8 vap_id = 0;

info = IEEE80211_SKB_CB(skb);
--
2.6.3


2016-01-15 13:12:22

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH v3 3/3] rsi: Replace variable initialisations by assignments in rsi_send_data_pkt()

From: Markus Elfring <[email protected]>
Date: Fri, 15 Jan 2016 13:40:22 +0100

Replace explicit initialisation for two local variables at the beginning
by assignments.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/net/wireless/rsi/rsi_91x_pkt.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_pkt.c b/drivers/net/wireless/rsi/rsi_91x_pkt.c
index 4322df1..2c18c01 100644
--- a/drivers/net/wireless/rsi/rsi_91x_pkt.c
+++ b/drivers/net/wireless/rsi/rsi_91x_pkt.c
@@ -26,12 +26,12 @@
*/
int rsi_send_data_pkt(struct rsi_common *common, struct sk_buff *skb)
{
- struct rsi_hw *adapter = common->priv;
+ struct rsi_hw *adapter;
struct ieee80211_hdr *tmp_hdr;
struct ieee80211_tx_info *info;
struct skb_info *tx_params;
struct ieee80211_bss_conf *bss;
- int status = -EINVAL;
+ int status;
u8 ieee80211_size = MIN_802_11_HDR_LEN;
u8 extnd_size;
__le16 *frame_desc;
@@ -41,8 +41,10 @@ int rsi_send_data_pkt(struct rsi_common *common, struct sk_buff *skb)
bss = &info->control.vif->bss_conf;
tx_params = (struct skb_info *)info->driver_data;

- if (!bss->assoc)
+ if (!bss->assoc) {
+ status = -EINVAL;
goto err;
+ }

tmp_hdr = (struct ieee80211_hdr *)&skb->data[0];
seq_num = (le16_to_cpu(tmp_hdr->seq_ctrl) >> 4);
@@ -97,7 +99,7 @@ int rsi_send_data_pkt(struct rsi_common *common, struct sk_buff *skb)
frame_desc[7] = cpu_to_le16(((tx_params->tid & 0xf) << 4) |
(skb->priority & 0xf) |
(tx_params->sta_id << 8));
-
+ adapter = common->priv;
status = adapter->host_intf_write_pkt(common->priv,
skb->data,
skb->len);
--
2.6.3


2016-01-02 16:33:34

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 1/3] rsi: Delete unnecessary variable initialisations in rsi_send_mgmt_pkt()

Hi Markus,

[auto build test WARNING on wireless-drivers-next/master]
[also build test WARNING on v4.4-rc7 next-20151231]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url: https://github.com/0day-ci/linux/commits/SF-Markus-Elfring/net-rsi-Fine-tuning-for-two-function-implementations/20160102-224740
base: https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
config: x86_64-randconfig-s2-01030012 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

drivers/net/wireless/rsi/rsi_91x_pkt.c: In function 'rsi_send_mgmt_pkt':
>> drivers/net/wireless/rsi/rsi_91x_pkt.c:211:2: warning: 'status' may be used uninitialized in this function [-Wmaybe-uninitialized]
rsi_indicate_tx_status(common->priv, skb, status);
^

vim +/status +211 drivers/net/wireless/rsi/rsi_91x_pkt.c

dad0d04f Fariya Fatima 2014-03-16 195 /* Indicate to firmware to give cfm */
dad0d04f Fariya Fatima 2014-03-16 196 if ((skb->data[16] == IEEE80211_STYPE_PROBE_REQ) && (!bss->assoc)) {
dad0d04f Fariya Fatima 2014-03-16 197 msg[1] |= cpu_to_le16(BIT(10));
dad0d04f Fariya Fatima 2014-03-16 198 msg[7] = cpu_to_le16(PROBEREQ_CONFIRM);
dad0d04f Fariya Fatima 2014-03-16 199 common->mgmt_q_block = true;
dad0d04f Fariya Fatima 2014-03-16 200 }
dad0d04f Fariya Fatima 2014-03-16 201
dad0d04f Fariya Fatima 2014-03-16 202 msg[7] |= cpu_to_le16(vap_id << 8);
dad0d04f Fariya Fatima 2014-03-16 203
dad0d04f Fariya Fatima 2014-03-16 204 status = adapter->host_intf_write_pkt(common->priv,
dad0d04f Fariya Fatima 2014-03-16 205 (u8 *)msg,
dad0d04f Fariya Fatima 2014-03-16 206 skb->len);
dad0d04f Fariya Fatima 2014-03-16 207 if (status)
dad0d04f Fariya Fatima 2014-03-16 208 rsi_dbg(ERR_ZONE, "%s: Failed to write the packet\n", __func__);
dad0d04f Fariya Fatima 2014-03-16 209
dad0d04f Fariya Fatima 2014-03-16 210 err:
dad0d04f Fariya Fatima 2014-03-16 @211 rsi_indicate_tx_status(common->priv, skb, status);
dad0d04f Fariya Fatima 2014-03-16 212 return status;
dad0d04f Fariya Fatima 2014-03-16 213 }

:::::: The code at line 211 was first introduced by commit
:::::: dad0d04fa7ba41ce603a01e8e64967650303e9a2 rsi: Add RS9113 wireless driver

:::::: TO: Fariya Fatima <[email protected]>
:::::: CC: John W. Linville <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (2.72 kB)
.config.gz (26.76 kB)
Download all attachments