On Tue, 2011-09-06 at 11:27 -0400, Christoph Hellwig wrote: > > > + xfs_extnum_t lastx; /* last useful extent number */ > > > + int eof; /* we've hit the end of extents */ > > > + int n = 0; /* current extent index */ > > > + int error = 0; > > > + > > > + ASSERT(*nmap >= 1); > > > + ASSERT(*nmap <= XFS_BMAP_MAX_NMAP); > > > + ASSERT(!(flags & ~XFS_BMAPI_ENTIRE)); > > > + > > > > Rearrange the following test to use the pattern (assigning error) > > used in xfs_bmapi_read(). > > Hmm - given that error is used as a boolean there I don't actually like > that pattern very much as error is generally used to hold an errno > value. My main thought was "make them consistent." But looking at it again I agree that "error" is not the right name. It doesn't really need changing but could be beautified in some later patch. > > > > > + if (unlikely(XFS_TEST_ERROR( > > > + (XFS_IFORK_FORMAT(ip, XFS_DATA_FORK) != XFS_DINODE_FMT_EXTENTS && > > > + XFS_IFORK_FORMAT(ip, XFS_DATA_FORK) != XFS_DINODE_FMT_BTREE), > > > > Are you certain that XFS_DINODE_FMT_LOCAL is not possible here? > > I tried to trace it back but I'm still not sure. The transaction > > pointer passed is null, so it would have tripped an assertion > > in the previous code. (A simple explanation would reassure me.) > > We can't hit it because we do not support the local format for regular > files at all, and we do not support delayed allocations for anything > but regular files. OK. That's another thing I didn't realize about XFS... -Alex _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
