On Mon, Feb 09, 2015 at 08:09:26AM -0500, Brian Foster wrote: > On Mon, Feb 09, 2015 at 08:35:02AM +1100, Dave Chinner wrote: > > On Fri, Feb 06, 2015 at 04:22:04PM -0600, Eric Sandeen wrote: > > > These 2 patches provide information about which filesystem > > > hit the error... > > > > If we are going to touch every one of these macros, then can we > > rename them to something a little shorter like XFS_CORRUPT_GOTO() > > and XFS_CORRUPT_RETURN() at the same time? That will make the code a > > little less eye-bleedy where there are lots of these statements, > > and make formatting of complex checks a bit easier, too... > > > > XFS_CORRUPT_DOSOMETHING() jumps out to me as indicate corruption if the > logic statement evaluates as true rather than false. The latter (e.g., > assert-like logic) is how they work today, so that could be a bit > confusing to somebody who isn't already familiar with how these macros > work. Someone not familiar with XFS conventions is already going to get caught by "should be true" logic of these statements anyway as the logic is the opposite of BUG_ON() and WARN_ON(). i.e. BUG_ON(1) will kill the kernel, while ASSERT(1) indicates everything is fine. I suggested shortening the macro because it makes the code that uses it extensively shouty and hard to read because it splits logic statements across lines regularly (e.g __xfs_dir3_data_check). I want to use this more extensively in verifiers to give better corruption detection reporting, but the current macro will make the verifier code rather ugly. Hence my suggestion to make it shorter, neater and a little less shouty... > Unfortunately, nothing shorter than the current naming immediately comes > to mind... :/ We could kill the XFS_ prefix I suppose or even invert the > logic of the calls, but that's certainly a more significant change. > Thoughts? No logic changes, please. Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
