* [PATCH 0/2] usbip: Adjustments for two function implementations
@ 2017-12-05 21:48 SF Markus Elfring
2017-12-05 21:49 ` [PATCH 1/2] usbip: Delete an error message for a failed memory allocation in two functions SF Markus Elfring
2017-12-05 21:52 ` [PATCH 2/2] usbip: Use common error handling code in stub_recv_cmd_submit() SF Markus Elfring
0 siblings, 2 replies; 6+ messages in thread
From: SF Markus Elfring @ 2017-12-05 21:48 UTC (permalink / raw)
To: linux-usb, Greg Kroah-Hartman, Shuah Khan, Valentina Manea
Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 5 Dec 2017 22:44:55 +0100
Two update suggestions were taken into account
from static source code analysis.
Markus Elfring (2):
Delete an error message for a failed memory allocation in two functions
Use common error handling code in stub_recv_cmd_submit()
drivers/usb/usbip/stub_rx.c | 24 ++++++++++--------------
1 file changed, 10 insertions(+), 14 deletions(-)
--
2.15.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] usbip: Delete an error message for a failed memory allocation in two functions
2017-12-05 21:48 [PATCH 0/2] usbip: Adjustments for two function implementations SF Markus Elfring
@ 2017-12-05 21:49 ` SF Markus Elfring
2017-12-05 22:14 ` Shuah Khan
2017-12-05 21:52 ` [PATCH 2/2] usbip: Use common error handling code in stub_recv_cmd_submit() SF Markus Elfring
1 sibling, 1 reply; 6+ messages in thread
From: SF Markus Elfring @ 2017-12-05 21:49 UTC (permalink / raw)
To: linux-usb, Greg Kroah-Hartman, Shuah Khan, Valentina Manea
Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 5 Dec 2017 22:25:38 +0100
Omit an extra message for a memory allocation failure in these functions.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/usb/usbip/stub_rx.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/usb/usbip/stub_rx.c b/drivers/usb/usbip/stub_rx.c
index 536e037f541f..a70eb81823a3 100644
--- a/drivers/usb/usbip/stub_rx.c
+++ b/drivers/usb/usbip/stub_rx.c
@@ -302,7 +302,6 @@ static struct stub_priv *stub_priv_alloc(struct stub_device *sdev,
priv = kmem_cache_zalloc(stub_priv_cache, GFP_ATOMIC);
if (!priv) {
- dev_err(&sdev->udev->dev, "alloc stub_priv\n");
spin_unlock_irqrestore(&sdev->priv_lock, flags);
usbip_event_add(ud, SDEV_EVENT_ERROR_MALLOC);
return NULL;
@@ -466,7 +465,6 @@ static void stub_recv_cmd_submit(struct stub_device *sdev,
priv->urb->setup_packet = kmemdup(&pdu->u.cmd_submit.setup, 8,
GFP_KERNEL);
if (!priv->urb->setup_packet) {
- dev_err(&udev->dev, "allocate setup_packet\n");
usbip_event_add(ud, SDEV_EVENT_ERROR_MALLOC);
return;
}
--
2.15.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] usbip: Use common error handling code in stub_recv_cmd_submit()
2017-12-05 21:48 [PATCH 0/2] usbip: Adjustments for two function implementations SF Markus Elfring
2017-12-05 21:49 ` [PATCH 1/2] usbip: Delete an error message for a failed memory allocation in two functions SF Markus Elfring
@ 2017-12-05 21:52 ` SF Markus Elfring
2017-12-05 22:21 ` Shuah Khan
1 sibling, 1 reply; 6+ messages in thread
From: SF Markus Elfring @ 2017-12-05 21:52 UTC (permalink / raw)
To: linux-usb, Greg Kroah-Hartman, Shuah Khan, Valentina Manea
Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 5 Dec 2017 22:40:30 +0100
Add a jump target so that a bit of exception handling can be better reused
at the end of this function.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/usb/usbip/stub_rx.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/usb/usbip/stub_rx.c b/drivers/usb/usbip/stub_rx.c
index a70eb81823a3..f46d71abd2f9 100644
--- a/drivers/usb/usbip/stub_rx.c
+++ b/drivers/usb/usbip/stub_rx.c
@@ -445,29 +445,23 @@ static void stub_recv_cmd_submit(struct stub_device *sdev,
else
priv->urb = usb_alloc_urb(0, GFP_KERNEL);
- if (!priv->urb) {
- usbip_event_add(ud, SDEV_EVENT_ERROR_MALLOC);
- return;
- }
+ if (!priv->urb)
+ goto add_event_malloc_failure;
/* allocate urb transfer buffer, if needed */
if (pdu->u.cmd_submit.transfer_buffer_length > 0) {
priv->urb->transfer_buffer =
kzalloc(pdu->u.cmd_submit.transfer_buffer_length,
GFP_KERNEL);
- if (!priv->urb->transfer_buffer) {
- usbip_event_add(ud, SDEV_EVENT_ERROR_MALLOC);
- return;
- }
+ if (!priv->urb->transfer_buffer)
+ goto add_event_malloc_failure;
}
/* copy urb setup packet */
priv->urb->setup_packet = kmemdup(&pdu->u.cmd_submit.setup, 8,
GFP_KERNEL);
- if (!priv->urb->setup_packet) {
- usbip_event_add(ud, SDEV_EVENT_ERROR_MALLOC);
- return;
- }
+ if (!priv->urb->setup_packet)
+ goto add_event_malloc_failure;
/* set other members from the base header of pdu */
priv->urb->context = (void *) priv;
@@ -507,6 +501,10 @@ static void stub_recv_cmd_submit(struct stub_device *sdev,
}
usbip_dbg_stub_rx("Leave\n");
+ return;
+
+add_event_malloc_failure:
+ usbip_event_add(ud, SDEV_EVENT_ERROR_MALLOC);
}
/* recv a pdu */
--
2.15.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] usbip: Delete an error message for a failed memory allocation in two functions
2017-12-05 21:49 ` [PATCH 1/2] usbip: Delete an error message for a failed memory allocation in two functions SF Markus Elfring
@ 2017-12-05 22:14 ` Shuah Khan
2017-12-06 9:10 ` SF Markus Elfring
0 siblings, 1 reply; 6+ messages in thread
From: Shuah Khan @ 2017-12-05 22:14 UTC (permalink / raw)
To: SF Markus Elfring, linux-usb, Greg Kroah-Hartman, Valentina Manea
Cc: LKML, kernel-janitors, Shuah Khan, Shuah Khan
On 12/05/2017 02:49 PM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 5 Dec 2017 22:25:38 +0100
>
> Omit an extra message for a memory allocation failure in these functions.
>
> This issue was detected by using the Coccinelle software.
Please include the problem and log from Coccinelle software in any
future patches for the issues detected by Coccinelle.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> drivers/usb/usbip/stub_rx.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/usb/usbip/stub_rx.c b/drivers/usb/usbip/stub_rx.c
> index 536e037f541f..a70eb81823a3 100644
> --- a/drivers/usb/usbip/stub_rx.c
> +++ b/drivers/usb/usbip/stub_rx.c
> @@ -302,7 +302,6 @@ static struct stub_priv *stub_priv_alloc(struct stub_device *sdev,
>
> priv = kmem_cache_zalloc(stub_priv_cache, GFP_ATOMIC);
> if (!priv) {
> - dev_err(&sdev->udev->dev, "alloc stub_priv\n");
> spin_unlock_irqrestore(&sdev->priv_lock, flags);
> usbip_event_add(ud, SDEV_EVENT_ERROR_MALLOC);
> return NULL;
> @@ -466,7 +465,6 @@ static void stub_recv_cmd_submit(struct stub_device *sdev,
> priv->urb->setup_packet = kmemdup(&pdu->u.cmd_submit.setup, 8,
> GFP_KERNEL);
> if (!priv->urb->setup_packet) {
> - dev_err(&udev->dev, "allocate setup_packet\n");
If Coccinelle found this as an extra message, there is something
wrong with the Coccinelle script. This is not an extra message.
This message is for the second kmemdup() failure and is necessary.
> usbip_event_add(ud, SDEV_EVENT_ERROR_MALLOC);
> return;
> }
>
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] usbip: Use common error handling code in stub_recv_cmd_submit()
2017-12-05 21:52 ` [PATCH 2/2] usbip: Use common error handling code in stub_recv_cmd_submit() SF Markus Elfring
@ 2017-12-05 22:21 ` Shuah Khan
0 siblings, 0 replies; 6+ messages in thread
From: Shuah Khan @ 2017-12-05 22:21 UTC (permalink / raw)
To: SF Markus Elfring, linux-usb, Greg Kroah-Hartman, Valentina Manea
Cc: LKML, kernel-janitors, Shuah Khan, Shuah Khan
On 12/05/2017 02:52 PM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 5 Dec 2017 22:40:30 +0100
>
> Add a jump target so that a bit of exception handling can be better reused
> at the end of this function.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> drivers/usb/usbip/stub_rx.c | 22 ++++++++++------------
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/usb/usbip/stub_rx.c b/drivers/usb/usbip/stub_rx.c
> index a70eb81823a3..f46d71abd2f9 100644
> --- a/drivers/usb/usbip/stub_rx.c
> +++ b/drivers/usb/usbip/stub_rx.c
> @@ -445,29 +445,23 @@ static void stub_recv_cmd_submit(struct stub_device *sdev,
> else
> priv->urb = usb_alloc_urb(0, GFP_KERNEL);
>
> - if (!priv->urb) {
> - usbip_event_add(ud, SDEV_EVENT_ERROR_MALLOC);
> - return;
> - }
> + if (!priv->urb)
> + goto add_event_malloc_failure;
>
> /* allocate urb transfer buffer, if needed */
> if (pdu->u.cmd_submit.transfer_buffer_length > 0) {
> priv->urb->transfer_buffer =
> kzalloc(pdu->u.cmd_submit.transfer_buffer_length,
> GFP_KERNEL);
> - if (!priv->urb->transfer_buffer) {
> - usbip_event_add(ud, SDEV_EVENT_ERROR_MALLOC);
> - return;
> - }
> + if (!priv->urb->transfer_buffer)
> + goto add_event_malloc_failure;
> }
>
> /* copy urb setup packet */
> priv->urb->setup_packet = kmemdup(&pdu->u.cmd_submit.setup, 8,
> GFP_KERNEL);
> - if (!priv->urb->setup_packet) {
> - usbip_event_add(ud, SDEV_EVENT_ERROR_MALLOC);
Please see comments on my comments on the previous patch in this series.
This patch depends on the patch that is incorrect.
> - return;
> - }
> + if (!priv->urb->setup_packet)
> + goto add_event_malloc_failure;
>
> /* set other members from the base header of pdu */
> priv->urb->context = (void *) priv;
> @@ -507,6 +501,10 @@ static void stub_recv_cmd_submit(struct stub_device *sdev,
> }
>
> usbip_dbg_stub_rx("Leave\n");
> + return;
> +
> +add_event_malloc_failure:
> + usbip_event_add(ud, SDEV_EVENT_ERROR_MALLOC);
> }
>
> /* recv a pdu */
>
I am restructuring this routine to fix a bug and so it stands now,
I don't see a value in taking this patch that doesn't really fix
any specific bug.
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] usbip: Delete an error message for a failed memory allocation in two functions
2017-12-05 22:14 ` Shuah Khan
@ 2017-12-06 9:10 ` SF Markus Elfring
0 siblings, 0 replies; 6+ messages in thread
From: SF Markus Elfring @ 2017-12-06 9:10 UTC (permalink / raw)
To: shuah, linux-usb
Cc: Greg Kroah-Hartman, Shuah Khan, Valentina Manea, kernel-janitors, LKML
>> Omit an extra message for a memory allocation failure in these functions.
>>
>> This issue was detected by using the Coccinelle software.
>
> Please include the problem
Do you find the wording “WARNING: Possible unnecessary 'out of memory' message”
from the script “checkpatch.pl” more reasonable?
> and log from Coccinelle software
There is no log file from which I could extract something for this case.
> in any future patches for the issues detected by Coccinelle.
Would you like to help a bit to make the commit message better for your needs?
>> @@ -466,7 +465,6 @@ static void stub_recv_cmd_submit(struct stub_device *sdev,
>> priv->urb->setup_packet = kmemdup(&pdu->u.cmd_submit.setup, 8,
>> GFP_KERNEL);
>> if (!priv->urb->setup_packet) {
>> - dev_err(&udev->dev, "allocate setup_packet\n");
>
> If Coccinelle found this as an extra message,
> there is something wrong with the Coccinelle script.
The source code analysis approach could be improved somehow.
> This is not an extra message.
There can be different opinions around the handling of such exceptional situations.
> This message is for the second kmemdup() failure and is necessary.
This function is called only once within the implementation of
the function “stub_recv_cmd_submit” (and in this source file).
Do you find a default Linux allocation failure report insufficient?
Regards,
Markus
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-12-06 9:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-05 21:48 [PATCH 0/2] usbip: Adjustments for two function implementations SF Markus Elfring
2017-12-05 21:49 ` [PATCH 1/2] usbip: Delete an error message for a failed memory allocation in two functions SF Markus Elfring
2017-12-05 22:14 ` Shuah Khan
2017-12-06 9:10 ` SF Markus Elfring
2017-12-05 21:52 ` [PATCH 2/2] usbip: Use common error handling code in stub_recv_cmd_submit() SF Markus Elfring
2017-12-05 22:21 ` Shuah Khan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).