From: SF Markus Elfring <hidden> Date: 2016-01-02 14:40:55
From: Markus Elfring <redacted>
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
From: SF Markus Elfring <hidden> Date: 2016-01-02 14:43:24
From: Markus Elfring <redacted>
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 <redacted>
---
drivers/net/wireless/rsi/rsi_91x_pkt.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
From: SF Markus Elfring <hidden> Date: 2016-01-02 14:44:34
From: Markus Elfring <redacted>
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 <redacted>
---
drivers/net/wireless/rsi/rsi_91x_pkt.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
From: SF Markus Elfring <hidden> Date: 2016-01-02 14:45:25
From: Markus Elfring <redacted>
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 <redacted>
---
drivers/net/wireless/rsi/rsi_91x_pkt.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
From: Markus Elfring <redacted>
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
From: SF Markus Elfring <hidden> Date: 2016-01-02 18:27:49
From: Markus Elfring <redacted>
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 <redacted>
---
drivers/net/wireless/rsi/rsi_91x_pkt.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
From: Dan Carpenter <hidden> Date: 2016-01-04 09:29:10
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
From: Dan Carpenter <hidden> Date: 2016-01-04 09:38:38
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
From: SF Markus Elfring <hidden> Date: 2016-01-04 10:44:30
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
From: Dan Carpenter <hidden> Date: 2016-01-04 11:48:58
On Mon, Jan 04, 2016 at 11:44:15AM +0100, SF Markus Elfring wrote:
quoted
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
Hi Markus,
On Mon, Jan 4, 2016 at 11:33 PM, SF Markus Elfring
[off-list ref] wrote:
quoted
quoted
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: julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Profile: http://www.google.com/profiles/julian.calaby/
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: SF Markus Elfring <hidden> Date: 2016-01-05 08:30:21
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
Hi Markus,
On Tue, Jan 5, 2016 at 7:29 PM, SF Markus Elfring
[off-list ref] wrote:
quoted
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.)
quoted
"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.
quoted
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: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
From: SF Markus Elfring <hidden> Date: 2016-01-05 16:23:23
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
From: SF Markus Elfring <hidden> Date: 2016-01-15 13:05:04
From: Markus Elfring <redacted>
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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: SF Markus Elfring <hidden> Date: 2016-01-15 13:09:31
From: Markus Elfring <redacted>
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 <redacted>
---
drivers/net/wireless/rsi/rsi_91x_pkt.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
From: SF Markus Elfring <hidden> Date: 2016-01-15 13:10:48
From 017d1bb49f46266ffeb33178ddd3022d6b341d71 Mon Sep 17 00:00:00 2001
From: Markus Elfring <redacted>
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 <redacted>
---
drivers/net/wireless/rsi/rsi_91x_pkt.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
From: SF Markus Elfring <hidden> Date: 2016-01-15 13:12:25
From: Markus Elfring <redacted>
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 <redacted>
---
drivers/net/wireless/rsi/rsi_91x_pkt.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)