Re: [Qemu-devel] [PATCH] dump.c: Fix memory leak issue in cleanup processing for dump_init()

[Chen Gang <gang.chen.5i5j@gmail.com>]

On 08/12/2014 11:43 PM, Laszlo Ersek wrote:
> On 08/03/14 17:28, Chen Gang wrote:
>> In dump_init(), when failure occurs, need notice about 'fd' and memory
>> mapping. So call dump_cleanup() for it (need let all initializations at
>> front).
>>
>> Also simplify dump_cleanup(): remove redundant 'ret' and redundant 'fd'
>> checking.
>>
>> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
>> ---
>>
[...]
> 
> The patch is not trivial at all, because the lifecycles of the affected
> resources are not trivial.
> 

OK, thanks.

For me, next, I shall split it into 2 pieces, one for resource leak, and
the other for improving current code, which may let reviewers easier
(maybe 2 patches belong to 2 different maintainers).

> The commit message is an abomination. If you want to contribute to open
> source, please learn proper English, and make a *serious* effort to
> document your changes. Don't expect earlier contributors to have any
> working knowledge about a file they have *maybe* touched several months
> ago. People have to swap out a whole load of crap from their minds, and
> swap in the old crap, to understand your patch. Help them by writing
> good commit messages.
> 

OK, thanks, And I am just trying to be improving.

 - always communicate with open source (qemu/kvm/kernel/binutils/gcc) in
   English (it must be).

 - I read/listen to Holy Bible in English every day: morning and evening
   in subway (I spend 2 hours from home to office, so 4 hours total),
   so at least, I spend about 1-1.5 hours for reading/listening per day.

> That said, no matter how annoying this submission is, my conscience
> didn't allow me to let it go ignored, so I spent the time and made an
> effort to track the lifetimes of "s->fd" and "s->list" in, and around,
> dump_init().
> 

OK, thanks, next, you can notify me for it, and I shall try to give some
related test (or we can test together for it), although it is not quite
easy for me.

> The patch seems correct. That's your only excuse for the loss of gray
> matter I've suffered while parsing your commit message.
> 
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> 

OK, thank you very much.


Thanks.
-- 
Chen Gang

Open share and attitude like air water and life which God blessed