Re: [PATCH] libceph: fix a memory leak in handle_watch_notify

[Ilya Dryomov <ilya.dryomov@inktank.com>]

On Thu, Sep 11, 2014 at 2:50 PM, Alex Elder <elder@ieee.org> wrote:
> On 09/11/2014 03:31 AM, Ilya Dryomov wrote:
>>
>> On Thu, Sep 11, 2014 at 5:41 AM, Alex Elder <elder@ieee.org> wrote:
>>>
>>> On 09/10/2014 07:20 PM, roy.qing.li@gmail.com wrote:
>>>>
>>>>
>>>> From: Li RongQing <roy.qing.li@gmail.com>
>>>>
>>>> event_work should be freed when adding it to queue failed
>>>>
>>>> Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
>>>
>>>
>>>
>>> Looks good.
>>>
>>> Reviewed-by: Alex Elder <elder@linaro.org>
>>
>>
>> Hmm, queue_work() returns %false if @work was already on a queue, %true
>> otherwise, so this seems bogus to me.  I'd go with something like this
>> (mangled).
>
>
> The original change was fine.  Whether it matters is another question.
> Your suggestion looks good as well, and on the assumption that if you
> choose to use it instead your "real" fix is done correctly you can
> use "Reviewed-by: <me>" if you like.

Well, the original change makes something bogus even more bogus.  It's
basically:

    foo = kmalloc(...);
    foo->bar = 0;

    if (foo->bar & BAZ) {
        /* WARNING */
        kfree(foo);
        goto ...
    }

So yeah, I'm going to use your Reviewed-by on my "real" fix ;)

Thanks,

                Ilya