On Fri, Dec 10, 2010 at 03:12:41PM +0800, Li Zefan wrote: > >> + /* nothing to do */ > >> + if (!!(flags & BTRFS_SUBVOL_RDONLY) == root->readonly) > >> + goto out_unlock; > > > > This is only an aesthetic comment: I prefer a simpler style like > > > > if ((flags & BTRFS_SUBVOL_RDONLY) && root->readonly) > > goto out_unlock; > > They are not equivalent. The former checks if the flags and the > root both have readonly bit set or cleared.
This is an excellent reason to add what you just replied (above) as a comment into the code ahead of that line, so others don't make the same mistake in interpreting what it does: /* nothing to do */ /* check if flags and root both have readonly bit set or cleared */ if (!!(flags & BTRFS_SUBVOL_RDONLY) == root->readonly) goto out_unlock; "Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it." - Brian W. Kernighan -- | Ian! D. Allen - idal...@idallen.ca - Ottawa, Ontario, Canada | Home Page: http://idallen.com/ Contact Improv: http://contactimprov.ca/ | College professor (Free/Libre GNU+Linux) at: http://teaching.idallen.com/ | Defend digital freedom: http://eff.org/ and have fun: http://fools.ca/ -- 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