>> > 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 at 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