lkml.org 
[lkml]   [2008]   [May]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 2/2] Eliminate double kfree
Hi Julia,

On Thu, May 29, 2008 at 03:05:07PM +0200, Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
>
> The variable report is only non-NULL and non-freed in a small region of
> code, so it should only be freed in error handling code that comes from
> that region.
>

Thank you for your patch. There isn't a double-kfree though, as far
as I can see. Because we need to free the report in both success and
failure cases the error handling is a bit unwieldy I agree. I don't
want to apply your patch though because I don't like when we bypass
parts of error handling path that weren't bypassed if we aborted
earlier, if you follow me.

What do you think about the patch below?

--
Dmitry

Input: gtco - clean up error handling in gtco_probe

Thanks to Julia Lawall for noticing ugliness.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
drivers/input/tablet/gtco.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
Index: linux/drivers/input/tablet/gtco.c
===================================================================
--- linux.orig/drivers/input/tablet/gtco.c
+++ linux/drivers/input/tablet/gtco.c
@@ -830,7 +830,7 @@ static int gtco_probe(struct usb_interfa
struct gtco *gtco;
struct input_dev *input_dev;
struct hid_descriptor *hid_desc;
- char *report = NULL;
+ char *report;
int result = 0, retry;
int error;
struct usb_endpoint_descriptor *endpoint;
@@ -916,12 +916,16 @@ static int gtco_probe(struct usb_interfa
le16_to_cpu(hid_desc->wDescriptorLength),
5000); /* 5 secs */

- if (result == le16_to_cpu(hid_desc->wDescriptorLength))
+ dbg("usb_control_msg result: %d", result);
+ if (result == le16_to_cpu(hid_desc->wDescriptorLength)) {
+ parse_hid_report_descriptor(gtco, report, result);
break;
+ }
}

+ kfree(report);
+
/* If we didn't get the report, fail */
- dbg("usb_control_msg result: :%d", result);
if (result != le16_to_cpu(hid_desc->wDescriptorLength)) {
err("Failed to get HID Report Descriptor of size: %d",
hid_desc->wDescriptorLength);
@@ -929,12 +933,6 @@ static int gtco_probe(struct usb_interfa
goto err_free_urb;
}

- /* Now we parse the report */
- parse_hid_report_descriptor(gtco, report, result);
-
- /* Now we delete it */
- kfree(report);
-
/* Create a device file node */
usb_make_path(gtco->usbdev, gtco->usbpath, sizeof(gtco->usbpath));
strlcat(gtco->usbpath, "/input0", sizeof(gtco->usbpath));
@@ -988,7 +986,6 @@ static int gtco_probe(struct usb_interfa
usb_buffer_free(gtco->usbdev, REPORT_MAX_SIZE,
gtco->buffer, gtco->buf_dma);
err_free_devs:
- kfree(report);
input_free_device(input_dev);
kfree(gtco);
return error;

\
 
 \ /
  Last update: 2008-05-29 17:37    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site