diff mbox

[OPW,kernel] staging:rtl8187se: Removed assignments from if statements in ieee80211/ieee80211_softmac.c.

Message ID 1394262360-5521-1-git-send-email-fempsci@gmail.com
State New, archived
Headers show

Commit Message

Chi Pham March 8, 2014, 7:06 a.m. UTC
Signed-off-by: Chi Pham <fempsci@gmail.com>
---
 .../rtl8187se/ieee80211/ieee80211_softmac.c        |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Sarah Sharp March 8, 2014, 8 p.m. UTC | #1
I'm pretty sure someone else already completed this change.  Greg
updated staging-next yesterday, please rebase your patches on top of
that.

On Sat, Mar 08, 2014 at 08:06:00AM +0100, Chi Pham wrote:
> Signed-off-by: Chi Pham <fempsci@gmail.com>
> ---
>  .../rtl8187se/ieee80211/ieee80211_softmac.c        |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8187se/ieee80211/ieee80211_softmac.c b/drivers/staging/rtl8187se/ieee80211/ieee80211_softmac.c
> index c27392d..da17214 100644
> --- a/drivers/staging/rtl8187se/ieee80211/ieee80211_softmac.c
> +++ b/drivers/staging/rtl8187se/ieee80211/ieee80211_softmac.c
> @@ -1809,7 +1809,8 @@ inline int ieee80211_rx_frame_softmac(struct ieee80211_device *ieee,
>  			if ((ieee->softmac_features & IEEE_SOFTMAC_ASSOCIATE) &&
>  				ieee->state == IEEE80211_ASSOCIATING_AUTHENTICATED &&
>  				ieee->iw_mode == IW_MODE_INFRA){
> -				if (0 == (errcode=assoc_parse(skb, &aid))){
> +				errcode = assoc_parse(skb, &aid);
> +				if (0 == errcode) {

Also, you might as well change that statement to

                                if (!errcode) {


>  					u16 left;
>  
>  					ieee->state=IEEE80211_LINKED;
> @@ -1900,7 +1901,8 @@ associate_complete:
>  
>  						IEEE80211_DEBUG_MGMT("Received authentication response");
>  
> -						if (0 == (errcode=auth_parse(skb, &challenge, &chlen))){
> +						errcode = auth_parse(skb, &challenge, &chlen);
> +						if (0 == errcode) {

Same here.

Sarah Sharp
Chi Pham March 8, 2014, 9:08 p.m. UTC | #2
On Saturday, March 8, 2014 9:00:56 PM UTC+1, Sarah Sharp wrote:
>
> I'm pretty sure someone else already completed this change.  Greg 
> updated staging-next yesterday, please rebase your patches on top of 
> that. 
>
>
Sorry, I'm not very good at git. I followed the 'Update your kernel' 
section of the first patch tutorial, ie. using git fetch origin and git 
rebase origin/staging-next, but there still seems to be a discrepancy 
between git log and git log origin/staging-next (the former has my commits, 
the latter does not).
How can I get the newest changes, and what will happen to the files I 
modified locally if they have been modified in the main branch?

Another question:
In all these 'remove assignments from if statements', I have used a 
coccinelle script. I do notice all the other small things that could be 
fixed in the affected code (such as 'if (!errcode)' which you mentioned), 
but should I make these changes as well as the coccinelle-induced changes 
in the same patch?
Sarah Sharp March 8, 2014, 9:47 p.m. UTC | #3
On Sat, Mar 08, 2014 at 01:08:27PM -0800, Chi Pham wrote:
> On Saturday, March 8, 2014 9:00:56 PM UTC+1, Sarah Sharp wrote:
> >
> > I'm pretty sure someone else already completed this change.  Greg 
> > updated staging-next yesterday, please rebase your patches on top of 
> > that. 
> >
> >
> Sorry, I'm not very good at git. I followed the 'Update your kernel' 
> section of the first patch tutorial, ie. using git fetch origin and git 
> rebase origin/staging-next, but there still seems to be a discrepancy 
> between git log and git log origin/staging-next (the former has my commits, 
> the latter does not).

That is expected.  If you run `gitk HEAD origin/staging-next`, you
should see your patches are on top of the last commit on staging-next.

> How can I get the newest changes, and what will happen to the files I 
> modified locally if they have been modified in the main branch?

Did `git rebase` actually run?  If you have modified but uncommitted
changes in your repo, it will refuse to rebase.

When you run that command, it will move the HEAD commit to the
staging-next HEAD commit, and attempt to apply your patches on top of
them.  If they apply successfully, you know you're up to date.
Sometimes a patch gets modified or dropped because someone already made
the same change.  I expect that this particular patch would just be
dropped.

What does `git log --pretty=oneline --abbrev-commit` show?

> Another question:
> In all these 'remove assignments from if statements', I have used a 
> coccinelle script. I do notice all the other small things that could be 
> fixed in the affected code (such as 'if (!errcode)' which you mentioned), 
> but should I make these changes as well as the coccinelle-induced changes 
> in the same patch? 

You can make that small change, but mention it in the commit message.
It's just that when you're already changing that line, it may make sense
to clean it up.

Sarah Sharp
Chi Pham March 8, 2014, 10:08 p.m. UTC | #4
On Saturday, March 8, 2014 10:47:26 PM UTC+1, Sarah Sharp wrote:
>
>
> Did `git rebase` actually run?  If you have modified but uncommitted 
> changes in your repo, it will refuse to rebase.  


> When you run that command, it will move the HEAD commit to the 
> staging-next HEAD commit, and attempt to apply your patches on top of 
> them.  If they apply successfully, you know you're up to date. 
> Sometimes a patch gets modified or dropped because someone already made 
> the same change.  I expect that this particular patch would just be 
> dropped. 
>

It did run without errors. I'm just not sure which of my patches have been 
dropped. This could be nice to know since I'm going to have to resend 
all/most of them anyway (since I split them up in different patches per 
file when they shouldn't be). 
 

> What does `git log --pretty=oneline --abbrev-commit` show? 
>

2f98fa0 staging:lustre: Removed assignments from if statements in 
lnet/klnds/soc
1d61f4f staging:lustre: Removed assignments from if statements in 
lnet/klnds/soc
2628430 staging:lustre: Removed assignments from if statements in 
osc/osc_quota.
021e5ac staging:lustre: Removed assignments from if statements in 
obdclass/obd_c
ec05052 staging:lustre: Removed assignments from if statements in 
mdc/mdc_reques
0368d35 staging:lustre: Removed assignments from if statements in 
lustre/lov/lov
7b8ba5e staging:lustre: Removed assignments from if statements in 
/lustre/llite/
b0702a2 staging:lustre: Removed assignments from if statements in 
lustre/libcfs/
d074e56 staging:lustre: Removed assignments from if statements in 
lustre/ptlrpc/
f5a51bd staging:rtl8188eu: Removed assignments in if statements in 
core/rtw_cmd.
f7b9f13 staging:rtl8187se: Removed assignments from if statements in 
ieee80211/i
5078107 staging:dgnc: Removed assignments from if statements in dgnc_tty.c.
2da3eb1 staging:dgnc: Removed assignments from if statements in dgnc_neo.c.
e2b6c9b staging:cxt1e1: Removed assignments in if statements in sbeproc.c
168e309 staging:cxt1e1: Removed assignments in if statements in pmcc4_drv.c
352fd65 staging:cxt1e1: Removed assignments in if statements in musycc.c
860aadd staging:cxt1e1: Removed assignments in if statements in linux.c
bf9aac5 staging:rtl8187se: Change argument type in function to bool
43f82a6 staging: rtl8187se: Delete typedef _ThreeWire
a983c95 staging: rtl8187se: Convert r8180_priv typedef into a struct
da37aab staging: rtl8187se: Convert buffer typedef into a struct
83efd52 Staging: rtl8192e: Fix Sparse Warning of invalid assignment in 
rtllib_tx
8a3efe9 staging: rtl8187se: Fix whitespaces in ieee80211/dot11d.h
Sarah Sharp March 9, 2014, 2:11 a.m. UTC | #5
On Sat, Mar 08, 2014 at 02:08:14PM -0800, Chi Pham wrote:
> On Saturday, March 8, 2014 10:47:26 PM UTC+1, Sarah Sharp wrote:
> >
> >
> > Did `git rebase` actually run?  If you have modified but uncommitted 
> > changes in your repo, it will refuse to rebase.  
> 
> 
> > When you run that command, it will move the HEAD commit to the 
> > staging-next HEAD commit, and attempt to apply your patches on top of 
> > them.  If they apply successfully, you know you're up to date. 
> > Sometimes a patch gets modified or dropped because someone already made 
> > the same change.  I expect that this particular patch would just be 
> > dropped. 
> >
> 
> It did run without errors. I'm just not sure which of my patches have been 
> dropped. This could be nice to know since I'm going to have to resend 
> all/most of them anyway (since I split them up in different patches per 
> file when they shouldn't be). 

Well, you should at least be able to tell from your subject lines which
ones need to get combined into one patch.  In the future, you can always
checkout a different branch, rebase, and see which patches were dropped.

You can probably still get back to your previous branch history, by
digging a bit.  If you run `git reflog`, you will get a log of your
branch history, and which git commands were used.  For example, say my
reflog shows:

fc17fe387718 HEAD@{0}: commit: Staging: tidspbridge: Fix split strings in drivers/staging/tidspbridge/core/tiomap3430.c
152cb4232c4f HEAD@{1}: commit (amend): Staging: tidspbridge: Fix split strings in drivers/staging/tidspbridge/core/io_sm.c
327f303c207c HEAD@{2}: commit: Staging: tidspbridge: Fix split strings.
4096ec993f10 HEAD@{3}: reset: moving to 4096ec993f10

What this shows is I reset my HEAD to commit 4096ec993f10, made a
commit, amended it, and then made another commit.
`git log --pretty=oneline --abbrev-commit` shows:

fc17fe387718 Staging: tidspbridge: Fix split strings in drivers/staging/tidspbridge/core/tiomap3430.c
152cb4232c4f Staging: tidspbridge: Fix split strings in drivers/staging/tidspbridge/core/io_sm.c
4096ec993f10 staging: rts5139: Fix quoted string split across lines

Then I decided to rebase my branch against the latest staging-next
branch.  Now git reflog shows:

de11fef9a1f9 HEAD@{0}: rebase finished: returning to refs/heads/master
de11fef9a1f9 HEAD@{1}: rebase: Staging: tidspbridge: Fix split strings in drivers/staging/tidspbridge/core/tiomap3430.c
ec16347cbd0b HEAD@{2}: rebase: Staging: tidspbridge: Fix split strings in drivers/staging/tidspbridge/core/io_sm.c
f3c25d569045 HEAD@{3}: checkout: moving from master to f3c25d569045c8e0f274956291b51641241da174^0
fc17fe387718 HEAD@{4}: commit: Staging: tidspbridge: Fix split strings in drivers/staging/tidspbridge/core/tiomap3430.c
152cb4232c4f HEAD@{5}: commit (amend): Staging: tidspbridge: Fix split strings in drivers/staging/tidspbridge/core/io_sm.c
327f303c207c HEAD@{6}: commit: Staging: tidspbridge: Fix split strings.
4096ec993f10 HEAD@{7}: reset: moving to 4096ec993f10

What git did was check out the tip commit on the remote staging-next
branch (commit f3c25d569045), and then applied my two commits on top of
that branch.  If I look at my git log, it now shows

de11fef9a1f9 Staging: tidspbridge: Fix split strings in drivers/staging/tidspbridge/core/tiomap3430.c
ec16347cbd0b Staging: tidspbridge: Fix split strings in drivers/staging/tidspbridge/core/io_sm.c
f3c25d569045 Staging: comedi: Fix 80-char line limit style issue in addi_apci_1500.c
11899188beec Staging: comedi: addi-data: tidy up counter register map defines in hwdrv_apci1564.c

If I messed up that rebase somehow, I can check out the HEAD commit I
had before the rebase, by running

git checkout -b redo fc17fe387718

Or I can just look at the list of commits I had before the rebase by
running

git log --pretty=oneline --abbrev-commit fc17fe387718

I suspect you could do something similar with your git reflog output.

> > What does `git log --pretty=oneline --abbrev-commit` show? 
> >
> 
> 2f98fa0 staging:lustre: Removed assignments from if statements in 
> lnet/klnds/soc
> 1d61f4f staging:lustre: Removed assignments from if statements in 
> lnet/klnds/soc
> 2628430 staging:lustre: Removed assignments from if statements in 
> osc/osc_quota.
> 021e5ac staging:lustre: Removed assignments from if statements in 
> obdclass/obd_c
> ec05052 staging:lustre: Removed assignments from if statements in 
> mdc/mdc_reques
> 0368d35 staging:lustre: Removed assignments from if statements in 
> lustre/lov/lov
> 7b8ba5e staging:lustre: Removed assignments from if statements in 
> /lustre/llite/
> b0702a2 staging:lustre: Removed assignments from if statements in 
> lustre/libcfs/
> d074e56 staging:lustre: Removed assignments from if statements in 
> lustre/ptlrpc/
> f5a51bd staging:rtl8188eu: Removed assignments in if statements in 
> core/rtw_cmd.
> f7b9f13 staging:rtl8187se: Removed assignments from if statements in 
> ieee80211/i
> 5078107 staging:dgnc: Removed assignments from if statements in dgnc_tty.c.
> 2da3eb1 staging:dgnc: Removed assignments from if statements in dgnc_neo.c.
> e2b6c9b staging:cxt1e1: Removed assignments in if statements in sbeproc.c
> 168e309 staging:cxt1e1: Removed assignments in if statements in pmcc4_drv.c
> 352fd65 staging:cxt1e1: Removed assignments in if statements in musycc.c
> 860aadd staging:cxt1e1: Removed assignments in if statements in linux.c
> bf9aac5 staging:rtl8187se: Change argument type in function to bool
> 43f82a6 staging: rtl8187se: Delete typedef _ThreeWire
> a983c95 staging: rtl8187se: Convert r8180_priv typedef into a struct
> da37aab staging: rtl8187se: Convert buffer typedef into a struct
> 83efd52 Staging: rtl8192e: Fix Sparse Warning of invalid assignment in 
> rtllib_tx
> 8a3efe9 staging: rtl8187se: Fix whitespaces in ieee80211/dot11d.h

You probably want to coerce your mail client into not line-wrapping
mails. :)  You can't use the gmail interface for replying to messages,
because it will mess things up, even when you reply in plain text
inline.

Well, I think you've basically got to revise and resend all your
patches, so you're going to need to use the -v2 flag to git-format-patch
for all of them anyway.  You can use `git rebase -i` to squash the
related patches together, that command is discussed a bit on
http://kernelnewbies.org/GitTips

Sarah Sharp
Greg KH March 9, 2014, 3:57 a.m. UTC | #6
On Sat, Mar 08, 2014 at 12:00:56PM -0800, Sarah Sharp wrote:
> I'm pretty sure someone else already completed this change.  Greg
> updated staging-next yesterday, please rebase your patches on top of
> that.

Nope, it applies just fine.
Chi Pham March 9, 2014, 9:23 a.m. UTC | #7
On Sunday, March 9, 2014 3:11:14 AM UTC+1, Sarah Sharp wrote:
>
> <snip>


That's extremely helpful, thanks a lot! 
I've been replying through the google groups interface comment section, 
because I wasn't able to send mails in the usual way.
But it seems my suspension has lifted, so now I'll try to see if I can get 
my patches up.

- Chi
diff mbox

Patch

diff --git a/drivers/staging/rtl8187se/ieee80211/ieee80211_softmac.c b/drivers/staging/rtl8187se/ieee80211/ieee80211_softmac.c
index c27392d..da17214 100644
--- a/drivers/staging/rtl8187se/ieee80211/ieee80211_softmac.c
+++ b/drivers/staging/rtl8187se/ieee80211/ieee80211_softmac.c
@@ -1809,7 +1809,8 @@  inline int ieee80211_rx_frame_softmac(struct ieee80211_device *ieee,
 			if ((ieee->softmac_features & IEEE_SOFTMAC_ASSOCIATE) &&
 				ieee->state == IEEE80211_ASSOCIATING_AUTHENTICATED &&
 				ieee->iw_mode == IW_MODE_INFRA){
-				if (0 == (errcode=assoc_parse(skb, &aid))){
+				errcode = assoc_parse(skb, &aid);
+				if (0 == errcode) {
 					u16 left;
 
 					ieee->state=IEEE80211_LINKED;
@@ -1900,7 +1901,8 @@  associate_complete:
 
 						IEEE80211_DEBUG_MGMT("Received authentication response");
 
-						if (0 == (errcode=auth_parse(skb, &challenge, &chlen))){
+						errcode = auth_parse(skb, &challenge, &chlen);
+						if (0 == errcode) {
 							if(ieee->open_wep || !challenge){
 								ieee->state = IEEE80211_ASSOCIATING_AUTHENTICATED;
 								ieee->softmac_stats.rx_auth_rs_ok++;