On 04/11/2018 02:32 AM, Qu Wenruo wrote: [...] >>>> so to get rid of generate_tab_indent and indent_str >>> >>> And we need to call such functions in each helper macros, with >>> duplicated codes. >> >> Please look at the asm generated: even if the "source generated" by the >> expansion of the macro is bigger, the binary code is smaller. >> E.g. the code below > > No, I don't mean asm code, but C code.
May be that there is some misunderstanding: my code is about 20loc, your one is about 50loc... I am missing something ? [...] >>> When passing random stream to dump-super, such reason will make output >>> quite nasty. >>> So just INVALID to info the user that some of the members don't look >>> valid is good enough, as the tool is only to help guys who are going to >>> manually patching superblocks. >> >> I think that we should increase the possible target also to who want to make >> some debugging :-) > > There are several problems here to output the condition > > 1) Loose condition > for basic alignment check it may looks good to output the condition, but > the fact is, the condition is not 100% correct for 64K pages system. > So when output IS_ALIGN(value, SZ_4K), it's not 100% correct. I don't understand your statement: does the alignment is the same for all the system ?. If not, this means that a filesystem created on a x86 might not work on a PPC64 (which IIRC is a 64k page hardware) ? > > 2) Too long condition for some case. > !(is_power_of_2(value) && value >= SZ_4K && value <= SZ_64K) > This seems pretty strange in fact. I agree that this is quite verbose... however if this is the constraint, why not print it > > 3) C macro usage > Just a minor problem, macro usage such as SZ_4K/SZ_64K may not looks > good for new developers. I find it quite clear and intuitive even if this is the first time that I notice these macros [...] > > Thanks, > Qu BR G.Baroncelli -- gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it> Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
