On 2019/9/30 下午9:34, Nikolay Borisov wrote: > > > 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. This is going to be a preference thing. To my taste, if possible, I would put ASSERT() in every function with more than 24 lines, as an alternative to comment, to show the prerequisite or the assumption. Thanks, Qu > >> >> 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,
