Re: [PATCH] cptm1217: check if interrupts are masked at probe

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jan 5, 2015 at 11:52 AM, devendra.aaru <devendra.aaru@xxxxxxxxx> wrote:
> On Mon, Jan 5, 2015 at 1:21 AM, sanjeev sharma
> <sanjeevsharmaengg@xxxxxxxxx> wrote:
>> On Mon, Jan 5, 2015 at 11:41 AM, devendra.aaru <devendra.aaru@xxxxxxxxx> wrote:
>>> On Mon, Jan 5, 2015 at 1:04 AM, sanjeev sharma
>>> <sanjeevsharmaengg@xxxxxxxxx> wrote:
>>>> On Fri, Jan 2, 2015 at 11:47 PM, Devendra Naga <devendra.aaru@xxxxxxxxx> wrote:
>>>>> the function cp_tm1217_mask_interrupt can return failure.
>>>>> added the check and the failure path.
>>>>>
>>>>> Cc: Ramesh Agarwal <ramesh.agarwal@xxxxxxxxx>
>>>>> Signed-off-by: Devendra Naga <devendra.aaru@xxxxxxxxx>
>>>>> ---
>>>>>
>>>>>  build tested only on x86_64. config is allmodconfig.
>>>>>
>>>>>  drivers/staging/cptm1217/clearpad_tm1217.c | 6 ++++++
>>>>>  1 file changed, 6 insertions(+)
>>>>>
>>>>> diff --git a/drivers/staging/cptm1217/clearpad_tm1217.c b/drivers/staging/cptm1217/clearpad_tm1217.c
>>>>> index 7f265ce..54e5953 100644
>>>>> --- a/drivers/staging/cptm1217/clearpad_tm1217.c
>>>>> +++ b/drivers/staging/cptm1217/clearpad_tm1217.c
>>>>> @@ -446,6 +446,12 @@ static int cp_tm1217_probe(struct i2c_client *client,
>>>>>
>>>>>         /* Mask all the interrupts */
>>>>>         retval = cp_tm1217_mask_interrupt(ts);
>>>>> +       if (retval) {
>>>>> +               dev_err(ts->dev, "failed to mask interrupts, error: %d\n",
>>>>> +                       retval);
>>>>> +               kfree(ts);
>>>>
>>>> Here you are doing more than what you have specified in change-log.How
>>>> did you find out memory leak ? Did you used any facility like kmemleak
>>>> facility to find this
>>>> problem ?
>>>
>>> No i am not doing more than one change. Read the change log one more time.
>>
>> you are addressing the memory leak which is not mentioned in change-log.
>
> i am pasting part of commit log here:
>
> commit log: "added the check and the failure path".
>
> Is this not enough to understand ?

you should have mentioned that memory is never released in previous
section of code hence freeing memory or you can also include "Fix
memory Leak" to make it clear what has been handled in the proposed
patch.IMO this should be the way and lets wait for Greg & other
opinion too.
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux