lkml.org 
[lkml]   [2012]   [Mar]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    SubjectRe: [PATCH] [media] staging: Return -EINTR in s2250_probe() if fails to get lock.
    From
    Oliver,

    The following changes are for review only not a formal patch.

    -------------------------------------------------------------------------------------------------------------
    - if (mutex_lock_interruptible(&usb->i2c_lock) == 0) {
    + mutex_lock(&usb->i2c_lock);
    data = kzalloc(16, GFP_KERNEL);
    - if (data != NULL) {
    + if(data == NULL) {
    + i2c_unregister_device(audio);
    + kfree(state);
    + return -ENOMEM;
    + } else {
    int rc;
    rc = go7007_usb_vendor_request(go, 0x41, 0, 0,
    data, 16, 1);
    @@ -657,7 +661,7 @@ static int s2250_probe(struct i2c_client *client,
    kfree(data);
    }
    mutex_unlock(&usb->i2c_lock);
    - }
    +

    ----------------------------------------------------------------------

    Is it ok ?


    regards
    Santosh


    On Fri, Mar 16, 2012 at 11:29 PM, Oliver Neukum <oliver@neukum.org> wrote:
    > Am Freitag, 16. März 2012, 17:56:20 schrieb santosh prasad nayak:
    >> On Fri, Mar 16, 2012 at 10:02 PM, Oliver Neukum <oliver@neukum.org> wrote:
    >> >
    >> > Indeed there's a lot wrong here. The idea of having an interruptible
    >> > sleep in probe() is arcane. You need a very, very, very good reason for that.
    >>
    >> Can you please explain why interruptible  sleep  should not be  in probe() ?
    >> I am curious to know.
    >
    > -EINTR is supposed to be returned to user space, so that it can repeat
    > an interrupted syscall.
    >
    > - There is no user space for probe()
    > - probe() cannot be easily repeated from user space
    > - there is no syscall for probe
    >>
    >>
    >> > The sane fix is using an uninterruptable sleep here.
    >> >
    >> > Second, while you are at it, fix the error case for no initialization
    >> > due to a failing kmalloc(). You need to return -ENOMEM.
    >>
    >> Are you talking about kmalloc or kzalloc ?
    >> Because for failing kmalloc -ENOMEM is returned as shown below:
    >
    >                data = kzalloc(16, GFP_KERNEL);
    >                if (data != NULL) {
    >                        int rc;
    >                        rc = go7007_usb_vendor_request(go, 0x41, 0, 0,
    >                                                       data, 16, 1);
    >                        if (rc > 0) {
    >                                u8 mask;
    >                                data[0] = 0;
    >                                mask = 1<<5;
    >                                data[0] &= ~mask;
    >                                data[1] |= mask;
    >                                go7007_usb_vendor_request(go, 0x40, 0,
    >                                                          (data[1]<<8)
    >                                                          + data[1],
    >                                                          data, 16, 0);
    >                        }
    >                        kfree(data);
    >                }
    >                mutex_unlock(&usb->i2c_lock)
    >
    > This code has no error handling.
    >
    >        Regards
    >                Oliver
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2012-03-17 17:03    [W:0.027 / U:89.188 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site