RE: [PATCH 3/3] close_dump_bitmap: simplify logic

[Atsushi Kumagai <ats-kumagai@wm.jp.nec.com>]

>> > The boolean expression replicates the logic of open_dump_bitmap().
>> > It's simpler and less error-prone to simply check if fd_bitmap is
>> > valid.
>> >
>> > Signed-off-by: Martin Wilck <mwilck@suse.de>
>> > ---
>> >  makedumpfile.c | 3 +--
>> >  1 file changed, 1 insertion(+), 2 deletions(-)
>> >
>> > diff --git a/makedumpfile.c b/makedumpfile.c
>> > index 43278f1..771ab7c 100644
>> > --- a/makedumpfile.c
>> > +++ b/makedumpfile.c
>> > @@ -8579,8 +8579,7 @@ close_dump_file(void)
>> >  void
>> >  close_dump_bitmap(void)
>> >  {
>> > -	if (!info->working_dir && !info->flag_reassemble && !info-
>> > >flag_refiltering
>> > -	    && !info->flag_sadump && !info->flag_mem_usage)
>> > +	if (!info->fd_bitmap)
>>
>> Strictly speaking, zero is a valid FD. I can see that it is highly
>> unlikely to be the bitmap FD, but it would be a nice cleanup to
>> initialize fd_bitmap to a negative number and check info->fd_bitmap <
>> 0.
>> I'm just not sure where to put the initializition...
>
>
>> > OTOH I know I'm asking you to fix something that you didn't break.
>
>I had the same thought, and the same excuse not to address it in this
>patch set. If you grep makedumpfile.c for "fd_bitmap", you'll see many
>checks like "if (info->fd_bitmap)". I just followed that pattern for
>now.

I see, it would be better to make the checks strict on this occasion.
So, could you work for that cleanup before your three patches as an
additional cleanup patch ?


Thanks,
Atsushi Kumagai
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec