Skip to contentSkip to conversation

staging: ath6kl: cleanup in SEND_FRAME ioctl

PATCH In review Dan Carpenter 5 participants 9 messages Feb 20, 2011 – Feb 22, 2011

Subsystems (2)
Staging Kernel janitors
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/2 staging: ath6kl: cleanup in SEND_FRAME ioctl

The original code was written in a funny way where every statement was part of else if blocks. I broke them up into separate statements by adding breaks on failure conditions.

Signed-off-by Dan Carpenter <[email protected]>
drivers/staging/ath6kl/os/linux/ioctl.c +14 -12
index 5be8ea3..17ba543 100644
@@ -3140,29 +3140,31 @@ int ar6000_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 
         case AR6000_XIOCTL_OPT_SEND_FRAME:
         {
-        WMI_OPT_TX_FRAME_CMD optTxFrmCmd;
+            WMI_OPT_TX_FRAME_CMD optTxFrmCmd;
             u8 data[MAX_OPT_DATA_LEN];
 
             if (ar->arWmiReady = false) {
                 ret = -EIO;
-            } else if (copy_from_user(&optTxFrmCmd, userdata,
-                                      sizeof(optTxFrmCmd)))
-            {
+                break;
+            }
+
+            if (copy_from_user(&optTxFrmCmd, userdata, sizeof(optTxFrmCmd))) {
                 ret = -EFAULT;
-            } else if (copy_from_user(data,
-                                      userdata+sizeof(WMI_OPT_TX_FRAME_CMD)-1,
-                                      optTxFrmCmd.optIEDataLen))
-            {
+                break;
+            }
+
+            if (copy_from_user(data, userdata+sizeof(WMI_OPT_TX_FRAME_CMD) - 1,
+                                   optTxFrmCmd.optIEDataLen)) {
                 ret = -EFAULT;
-            } else {
-                ret = wmi_opt_tx_frame_cmd(ar->arWmi,
+                break;
+            }
+
+            ret = wmi_opt_tx_frame_cmd(ar->arWmi,
                                            optTxFrmCmd.frmType,
                                            optTxFrmCmd.dstAddr,
                                            optTxFrmCmd.bssid,
                                            optTxFrmCmd.optIEDataLen,
                                            data);
-            }
-
             break;
         }
         case AR6000_XIOCTL_WMI_SETRETRYLIMITS:
Acked-by on v1 1/2
#1 lore raw
Quoted 44 lines · Dan Carpenter

The original code was written in a funny way where every statement was part of else if blocks. I broke them up into separate statements by adding breaks on failure conditions.

Signed-off-by Dan Carpenter <[email protected]>
drivers/staging/ath6kl/os/linux/ioctl.c +14 -10
index 5be8ea3..17ba543 100644
@@ -3140,29 +3140,31 @@ int ar6000_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 
         case AR6000_XIOCTL_OPT_SEND_FRAME:
         {
-        WMI_OPT_TX_FRAME_CMD optTxFrmCmd;
+            WMI_OPT_TX_FRAME_CMD optTxFrmCmd;
             u8 data[MAX_OPT_DATA_LEN];
 
             if (ar->arWmiReady = false) {
                 ret = -EIO;
-            } else if (copy_from_user(&optTxFrmCmd, userdata,
-                                      sizeof(optTxFrmCmd)))
-            {
+                break;
+            }
+
+            if (copy_from_user(&optTxFrmCmd, userdata, sizeof(optTxFrmCmd))) {
                 ret = -EFAULT;
-            } else if (copy_from_user(data,
-                                      userdata+sizeof(WMI_OPT_TX_FRAME_CMD)-1,
-                                      optTxFrmCmd.optIEDataLen))
-            {
+                break;
+            }
+
+            if (copy_from_user(data, userdata+sizeof(WMI_OPT_TX_FRAME_CMD) - 1,
+                                   optTxFrmCmd.optIEDataLen)) {
                 ret = -EFAULT;
-            } else {
-                ret = wmi_opt_tx_frame_cmd(ar->arWmi,
+                break;
+            }
+
+            ret = wmi_opt_tx_frame_cmd(ar->arWmi,

drivers/staging/ath6kl/os/linux/ioctl.c int ar6000_ioctl(struct net_device *dev, struct ifreq *rq, int cmd) · line 3170

Quoted 10 lines
drivers/staging/ath6kl/os/linux/ioctl.c +0 -2
                                            optTxFrmCmd.frmType,
                                            optTxFrmCmd.dstAddr,
                                            optTxFrmCmd.bssid,
                                            optTxFrmCmd.optIEDataLen,
                                            data);
-            }
-
             break;
         }
         case AR6000_XIOCTL_WMI_SETRETRYLIMITS:

The cleanup is applicable to the rest of the ar6000_ioctl as well.

Acked-by Vipin Mehta <[email protected]>
#2 lore raw
In reply to Dan Carpenter
Quoted 6 lines · Vipin Mehta
Quoted 5 lines · Dan Carpenter

The original code was written in a funny way where every statement was part of else if blocks.  I broke them up into separate statements by adding breaks on failure conditions.

Signed-off-by Dan Carpenter <[email protected]>

Can you guys please join IRC? ...
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#3 lore raw
In reply to Dan Carpenter
Quoted 9 lines · Luis R. Rodriguez
Quoted 6 lines · Vipin Mehta
Quoted 5 lines · Dan Carpenter

The original code was written in a funny way where every statement was part of else if blocks.  I broke them up into separate statements by adding breaks on failure conditions.

Signed-off-by Dan Carpenter <[email protected]>

Can you guys please join IRC? ...

That is not a requirement for kernel development, sorry.

email works best, especially when there are time zone differences, and travel issues.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#4 lore raw
In reply to Dan Carpenter
Quoted 7 lines · Greg KH
Quoted 6 lines · Luis R. Rodriguez
Quoted 5 lines · Vipin Mehta
Quoted 4 lines · Dan Carpenter

The original code was written in a funny way where every statement was part of else if blocks.  I broke them up into separate statements by adding breaks on failure conditions.

Quoted 5 lines
Quoted 3 lines
Quoted 1 line
Quoted 1 line
Signed-off-by Dan Carpenter <[email protected]>

Can you guys please join IRC? ...

That is not a requirement for kernel development, sorry.

No, but I'm not asking you, I'm asking Vipin and other developers, using IRC is a good idea for internal coordination. I see no way Vipin checking with Joe if he had his first set of patches ready. We need to do major cleanup on the driver *now* and these sort of miscommunication is just irritating. How do you recommend we coordinate cleanup with random patches being ACKed in the process without proper coordination?

Quoted 2 lines

email works best, especially when there are time zone differences, and travel issues.

Sure, I understand this.

Luis
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#5 lore raw
In reply to Dan Carpenter
Quoted 8 lines · Luis R. Rodriguez
Quoted 7 lines · Greg KH
Quoted 6 lines · Luis R. Rodriguez
Quoted 5 lines · Vipin Mehta
Quoted 4 lines · Dan Carpenter

The original code was written in a funny way where every statement was part of else if blocks.  I broke them up into separate statements by adding breaks on failure conditions.

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
Quoted 1 line
Signed-off-by Dan Carpenter <[email protected]>

Can you guys please join IRC? ...

That is not a requirement for kernel development, sorry.

No, but I'm not asking you, I'm asking Vipin and other developers, using IRC is a good idea for internal coordination.

I agree, but that wasn't what you said here :)

Quoted 5 lines

I see no way Vipin checking with Joe if he had his first set of patches ready. We need to do major cleanup on the driver *now* and these sort of miscommunication is just irritating. How do you recommend we coordinate cleanup with random patches being ACKed in the process without proper coordination?

You get Joe and Vipin to work together and agree with what to do. You don't email 4 other people and 2 mailing lists with a request to join IRC :)

It's not like we don't do this kind of things all the time, and have been for years, it just takes a willingness for the people involved to work together. Hopefully that is the case here.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#6 lore raw
In reply to Dan Carpenter
Quoted 26 lines · Greg KH
Quoted 8 lines · Luis R. Rodriguez
Quoted 7 lines · Greg KH
Quoted 6 lines · Luis R. Rodriguez
Quoted 5 lines · Vipin Mehta
Quoted 4 lines · Dan Carpenter

The original code was written in a funny way where every statement was part of else if blocks.  I broke them up into separate statements by adding breaks on failure conditions.

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
Quoted 1 line
Signed-off-by Dan Carpenter <[email protected]>

Can you guys please join IRC? ...

That is not a requirement for kernel development, sorry.

No, but I'm not asking you, I'm asking Vipin and other developers, using IRC is a good idea for internal coordination.

I agree, but that wasn't what you said here :)

Quoted 5 lines

I see no way Vipin checking with Joe if he had his first set of patches ready. We need to do major cleanup on the driver *now* and these sort of miscommunication is just irritating. How do you recommend we coordinate cleanup with random patches being ACKed in the process without proper coordination?

Quoted 3 lines

You get Joe and Vipin to work together and agree with what to do.  You don't email 4 other people and 2 mailing lists with a request to join IRC :)

I've already told Vipin and friends about things, the purpose of e-mailing other lists is to get people interested in sending patches for ath6kl to help us coordinate better with them. If I only e-mail private people I would have missed Dan Carpenter who sent these patches out.

Quoted 3 lines

It's not like we don't do this kind of things all the time, and have been for years, it just takes a willingness for the people involved to work together.  Hopefully that is the case here.

The will is there, but:

1) Vipin told me he was going on vacation Friday, and yet he's ACKing patches now
2) Who is the new maintainer for ath6kl ? I'm still waiting for the respective patches from Vipin so I guess I'll do that now

Luis

#7 lore raw
In reply to Dan Carpenter
Quoted 10 lines · Luis R. Rodriguez
Quoted 9 lines · Greg KH
Quoted 8 lines · Luis R. Rodriguez
Quoted 7 lines · Greg KH
Quoted 6 lines · Luis R. Rodriguez
Quoted 5 lines · Vipin Mehta
Quoted 4 lines · Dan Carpenter

The original code was written in a funny way where every statement was part of else if blocks. I broke them up into separate statements by adding breaks on failure conditions.

Signed-off-by Dan Carpenter <[email protected]>

Greg, can I find out soonish please if you are going to apply these patches?

If not, I have patches to send.
If so, I'll wait until you apply them
and redo them.

#8 lore raw
In reply to Dan Carpenter
Quoted 6 lines · Joe Perches
Quoted 5 lines · Luis R. Rodriguez
Quoted 4 lines · Greg KH
Quoted 3 lines · Luis R. Rodriguez
Quoted 2 lines · Greg KH
Quoted 1 line · Luis R. Rodriguez

On Sun, Feb 20, 2011 at 7:18 AM, Vipin Mehta <[email protected]> wrote:

Quoted 8 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines · Dan Carpenter
Quoted 4 lines

The original code was written in a funny way where every statement was part of else if blocks. I broke them up into separate statements by adding breaks on failure conditions.

Signed-off-by Dan Carpenter <[email protected]>

Greg, can I find out soonish please if you are going to apply these patches?

They were sent on Sunday, yesterday was a holiday here, so please give me a chance to catch up...

Quoted 3 lines

If not, I have patches to send.
If so, I'll wait until you apply them
and redo them.

Yes, it looks like I will take these as Vipin acked them.

thanks,

greg k-h