> On Oct 30, 2017, at 7:32 AM, Artem Blagodarenko <artem.blagodarenko@xxxxxxxxx> wrote: > > dx_db structure is freed after fixing of PR_2_HTREE_BAD_ROOT > problem. Next code block use this structure to unerstand if leaf > is beeng processed. > > If dx_db is freed, then root block is being processed and if_leaf > need to be set to 0. > > Signed-off-by: Artem Blagodarenko <artem.blagodarenko@xxxxxxxxx> > --- > e2fsck/pass2.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/e2fsck/pass2.c b/e2fsck/pass2.c > index 09c16179..7bda37a7 100644 > --- a/e2fsck/pass2.c > +++ b/e2fsck/pass2.c > @@ -1109,7 +1109,7 @@ inline_read_fail: > ((fs->blocksize - (8 + dx_csum_size)) / > sizeof(struct ext2_dx_entry)))) > dx_db->type = DX_DIRBLOCK_NODE; > - is_leaf = (dx_db->type == DX_DIRBLOCK_LEAF); > + is_leaf = dx_db ? (dx_db->type == DX_DIRBLOCK_LEAF) : 0; Ah, this code is a bit confusing. From looking at this patch, and even looking at the code directly, I thought "the previous line is also accessing dx_db, so what is the problem?" However, there really IS a problem, because the previous line is part of a different if/else block. It would be good to add braces around the "else" part of the condition to make it clear that this is a separate block of code, which is why we typically ask for matching braces for both legs of an if/else block. It would also be good to set "dx_db = NULL" instead of setting it to "0". Cheers, Andreas
Attachment:
signature.asc
Description: Message signed with OpenPGP
