* [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
* 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 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
* [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 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
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