[patch 1/2] staging: ath6kl: cleanup in SEND_FRAME ioctl

[patch 1/2] staging: ath6kl: cleanup in SEND_FRAME ioctl

[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 <error27@gmail.com>

diff --git a/drivers/staging/ath6kl/os/linux/ioctl.c b/drivers/staging/ath6kl/os/linux/ioctl.c
index 5be8ea3..17ba543 100644
--- a/drivers/staging/ath6kl/os/linux/ioctl.c
+++ b/drivers/staging/ath6kl/os/linux/ioctl.c
@@ -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:

Re: [patch 1/2] staging: ath6kl: cleanup in SEND_FRAME ioctl

[Vipin Mehta]

On Sun, Feb 20, 2011 at 04:49:08AM -0800, Dan Carpenter wrote:
> 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 <error27@gmail.com>
> 
> diff --git a/drivers/staging/ath6kl/os/linux/ioctl.c b/drivers/staging/ath6kl/os/linux/ioctl.c
> index 5be8ea3..17ba543 100644
> --- a/drivers/staging/ath6kl/os/linux/ioctl.c
> +++ b/drivers/staging/ath6kl/os/linux/ioctl.c
> @@ -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:
The cleanup is applicable to the rest of the ar6000_ioctl as well.

Acked-by: Vipin Mehta <vipin.mehta@atheros.com>

Re: [patch 1/2] staging: ath6kl: cleanup in SEND_FRAME ioctl

[Luis R. Rodriguez]

On Sun, Feb 20, 2011 at 7:18 AM, Vipin Mehta <vmehta@atheros.com> wrote:
> On Sun, Feb 20, 2011 at 04:49:08AM -0800, Dan Carpenter wrote:
>> 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 <error27@gmail.com>

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

Re: [patch 1/2] staging: ath6kl: cleanup in SEND_FRAME ioctl

[Greg KH]

On Sun, Feb 20, 2011 at 08:00:38PM -0800, Luis R. Rodriguez wrote:
> On Sun, Feb 20, 2011 at 7:18 AM, Vipin Mehta <vmehta@atheros.com> wrote:
> > On Sun, Feb 20, 2011 at 04:49:08AM -0800, Dan Carpenter wrote:
> >> 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 <error27@gmail.com>
> 
> 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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 1/2] staging: ath6kl: cleanup in SEND_FRAME ioctl

[Luis R. Rodriguez]

On Sun, Feb 20, 2011 at 8:14 PM, Greg KH <greg@kroah.com> wrote:
> On Sun, Feb 20, 2011 at 08:00:38PM -0800, Luis R. Rodriguez wrote:
>> On Sun, Feb 20, 2011 at 7:18 AM, Vipin Mehta <vmehta@atheros.com> wrote:
>> > On Sun, Feb 20, 2011 at 04:49:08AM -0800, Dan Carpenter wrote:
>> >> 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 <error27@gmail.com>
>>
>> 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?

> 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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 1/2] staging: ath6kl: cleanup in SEND_FRAME ioctl

[Greg KH]

On Sun, Feb 20, 2011 at 08:45:44PM -0800, Luis R. Rodriguez wrote:
> On Sun, Feb 20, 2011 at 8:14 PM, Greg KH <greg@kroah.com> wrote:
> > On Sun, Feb 20, 2011 at 08:00:38PM -0800, Luis R. Rodriguez wrote:
> >> On Sun, Feb 20, 2011 at 7:18 AM, Vipin Mehta <vmehta@atheros.com> wrote:
> >> > On Sun, Feb 20, 2011 at 04:49:08AM -0800, Dan Carpenter wrote:
> >> >> 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 <error27@gmail.com>
> >>
> >> 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 :)

> 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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 1/2] staging: ath6kl: cleanup in SEND_FRAME ioctl

[Luis R. Rodriguez]

On Sun, Feb 20, 2011 at 9:02 PM, Greg KH <greg@kroah.com> wrote:
> On Sun, Feb 20, 2011 at 08:45:44PM -0800, Luis R. Rodriguez wrote:
>> On Sun, Feb 20, 2011 at 8:14 PM, Greg KH <greg@kroah.com> wrote:
>> > On Sun, Feb 20, 2011 at 08:00:38PM -0800, Luis R. Rodriguez wrote:
>> >> On Sun, Feb 20, 2011 at 7:18 AM, Vipin Mehta <vmehta@atheros.com> wrote:
>> >> > On Sun, Feb 20, 2011 at 04:49:08AM -0800, Dan Carpenter wrote:
>> >> >> 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 <error27@gmail.com>
>> >>
>> >> 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 :)
>
>> 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 :)

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.

> 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

Re: [patch 1/2] staging: ath6kl: cleanup in SEND_FRAME ioctl

[Joe Perches]

On Sun, 2011-02-20 at 21:04 -0800, Luis R. Rodriguez wrote:
> On Sun, Feb 20, 2011 at 9:02 PM, Greg KH <greg@kroah.com> wrote:
> > On Sun, Feb 20, 2011 at 08:45:44PM -0800, Luis R. Rodriguez wrote:
> >> On Sun, Feb 20, 2011 at 8:14 PM, Greg KH <greg@kroah.com> wrote:
> >> > On Sun, Feb 20, 2011 at 08:00:38PM -0800, Luis R. Rodriguez wrote:
> >> >> On Sun, Feb 20, 2011 at 7:18 AM, Vipin Mehta <vmehta@atheros.com> wrote:
> >> >> > On Sun, Feb 20, 2011 at 04:49:08AM -0800, Dan Carpenter wrote:
> >> >> >> 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 <error27@gmail.com>

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.

Re: [patch 1/2] staging: ath6kl: cleanup in SEND_FRAME ioctl

[Greg KH]

On Tue, Feb 22, 2011 at 08:42:09AM -0800, Joe Perches wrote:
> On Sun, 2011-02-20 at 21:04 -0800, Luis R. Rodriguez wrote:
> > On Sun, Feb 20, 2011 at 9:02 PM, Greg KH <greg@kroah.com> wrote:
> > > On Sun, Feb 20, 2011 at 08:45:44PM -0800, Luis R. Rodriguez wrote:
> > >> On Sun, Feb 20, 2011 at 8:14 PM, Greg KH <greg@kroah.com> wrote:
> > >> > On Sun, Feb 20, 2011 at 08:00:38PM -0800, Luis R. Rodriguez wrote:
> > >> >> On Sun, Feb 20, 2011 at 7:18 AM, Vipin Mehta <vmehta@atheros.com> wrote:
> > >> >> > On Sun, Feb 20, 2011 at 04:49:08AM -0800, Dan Carpenter wrote:
> > >> >> >> 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 <error27@gmail.com>
> 
> 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...

> 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