| Message ID | 20180709154048.GA32345@embeddedor.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: usb: hso: use swap macro in hso_kick_transmit | expand |
From: "Gustavo A. R. Silva" <gustavo@embeddedor.com> Date: Mon, 9 Jul 2018 10:40:48 -0500 > @@ -1748,14 +1747,11 @@ static void hso_kick_transmit(struct hso_serial *serial) > goto out; > > /* Switch pointers around to avoid memcpy */ > - temp = serial->tx_buffer; > - serial->tx_buffer = serial->tx_data; > - serial->tx_data = temp; > + swap(serial->tx_buffer, serial->tx_data); > serial->tx_data_count = serial->tx_buffer_count; > serial->tx_buffer_count = 0; > > - /* If temp is set, it means we switched buffers */ > - if (temp && serial->write_data) { > + if (serial->write_data) { I think temp can be NULL here, you have to retain this check.
Hi David, On 07/12/2018 12:58 AM, David Miller wrote: > From: "Gustavo A. R. Silva" <gustavo@embeddedor.com> > Date: Mon, 9 Jul 2018 10:40:48 -0500 > >> @@ -1748,14 +1747,11 @@ static void hso_kick_transmit(struct hso_serial *serial) >> goto out; >> >> /* Switch pointers around to avoid memcpy */ >> - temp = serial->tx_buffer; >> - serial->tx_buffer = serial->tx_data; >> - serial->tx_data = temp; >> + swap(serial->tx_buffer, serial->tx_data); >> serial->tx_data_count = serial->tx_buffer_count; >> serial->tx_buffer_count = 0; >> >> - /* If temp is set, it means we switched buffers */ >> - if (temp && serial->write_data) { >> + if (serial->write_data) { > > I think temp can be NULL here, you have to retain this check. > I see. Well, in that case I will check serial->tx_data instead. Thanks for the feedback. -- Gustavo
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c index de305ea..78d128e 100644 --- a/drivers/net/usb/hso.c +++ b/drivers/net/usb/hso.c @@ -1732,7 +1732,6 @@ static int hso_serial_ioctl(struct tty_struct *tty, /* starts a transmit */ static void hso_kick_transmit(struct hso_serial *serial) { - u8 *temp; unsigned long flags; int res; @@ -1748,14 +1747,11 @@ static void hso_kick_transmit(struct hso_serial *serial) goto out; /* Switch pointers around to avoid memcpy */ - temp = serial->tx_buffer; - serial->tx_buffer = serial->tx_data; - serial->tx_data = temp; + swap(serial->tx_buffer, serial->tx_data); serial->tx_data_count = serial->tx_buffer_count; serial->tx_buffer_count = 0; - /* If temp is set, it means we switched buffers */ - if (temp && serial->write_data) { + if (serial->write_data) { res = serial->write_data(serial); if (res >= 0) serial->tx_urb_used = 1;
Make use of the swap macro and remove unnecessary variable *temp*. This makes the code easier to read and maintain. Also, slightly refactor some code due to the removal of *temp*. This code was detected with the help of Coccinelle. Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> --- drivers/net/usb/hso.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)