Hello, Andreas. On Friday 21 March 2008 04:04:03 you wrote: > On Mar 21, 2008 00:37 +0300, Andrew Perepechko wrote: > > +#define REV_ASSERT(r) BUG_ON((rev) != 0 && (rev) != 1) > > Umm, "r" and "rev" are not consistent above... > Indeed. In the beginning I made this macro without arguments, then added this argument not too carefully. It should work anyway since it depends on rev, not r, but this needs to be fixed to make sense. > Since this assertion is only on the in-memory quota structure, it would > probably be better to have something like > > #define REV_R0 0x12340000 > #define REV_R1 0x12340001 > > and when unpacking the on-disk revision number or it with REV_OFFSET and > > #define REV_ASSERT(revno) BUG_ON((revno) != REV_R0 && (revno) != REV_R1) > > That detects the common case of memory being zeroed for some reason. It > will also easily detect if you aren't properly swabbing the revision > and unmasking the 0x1234000 from the in-memory structure. I think it is a good idea. We can't do a lot for swabbing checks since 0 is for old revision (and we can't change this) and 1 is a good enough swabbing check. But for internal consistency purposes in-memory 0x12340000 and 0x12340001 values look better than 0 and 1. > > Just a thought... some people may not like this idea, but I dislike > using "0" as a magic number for anything. > > Cheers, Andreas > -- > Andreas Dilger > Sr. Staff Engineer, Lustre Group > Sun Microsystems of Canada, Inc. > > Best. Andrew. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
