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    [W:0.028 / U:0.036 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site