Re: [PATCH 1/3] btrfs-progs: check/lowmem: Add check and repair for invalid inode generation

Nikolay Borisov
2019-09-30T13:34:37Z

On 30.09.19 г. 15:24 ч., Qu Wenruo wrote:
> Yes, the ASSERT() doesn't make much sense by itself.
>
> However I still believe it won't be a problem.

It won't be a problem but it feels wrong to have this assert this deep
into the call chain. IMO It should be put where it can trigger at the
earliest which seems to be in check_inode_item. That function assumes
it's working with an inode item and goes to dereference inode members so
if the type is wrong we'd crash there instead of in repair_inode_gen_lowmem.

>
> It's compiler's job to remove such dead ASSERT(), but for human reader,
> I still believe this ASSERT() could still make sense, especially when
> the caller or callee can get more and more complex.
>
> Thanks,