Re: [PATCH 0/8] xfsdump: Ouchie! My bleeding eyes!

[Brian Foster <bfoster@redhat.com>]

On Thu, Oct 29, 2015 at 09:35:42AM +1100, Dave Chinner wrote:
> On Wed, Oct 28, 2015 at 07:51:39AM -0400, Brian Foster wrote:
> > On Fri, Oct 16, 2015 at 12:44:53PM +1100, Dave Chinner wrote:
> > > Hi folks,
> > > 
> > > Turns out that changes to exported XFS headers in xfsprogs v4.2.0
> > > broke the xfsdump build. the XFS dump build was implicitly including
> > > the platform definitions calculated for the xfsprogs build and so
> > > removing them from the xfsprogs headers made xfsdump very unhappy.
> > > 
> > ...
> > > 
> > > So, now the code base is a little bit cleaner, a lot less dependent
> > > on the xfsprogs header files, compiles cleanly on xfsprogs 3.2.x and
> > > 4.x releases, can easily have asserts build in or excluded (distro
> > > packages need to use "export DEBUG=-DNDEBUG" to exclude asserts),
> > > passes xfstests with asserts enabled and disabled, and best of all
> > > the source code is a little less eye-bleedy.
> > > 
> > > I really don't expect anyone to review this closely - it's *huge*
> > > chunk of boring search/replace change:
> > > 
> > >  94 files changed, 2929 insertions(+), 2652 deletions(-)
> > > 
> > > but I would like people to comment on/ack the approach I've taken
> > > here. If nobody objects/cares, I'll then do a 3.1.6 release early
> > > next week....
> > > 
> > 
> > I sent some comments on patch 1, otherwise the rest looks reasonable to
> > me on a quick pass through. The only thing I noticed is that the series
> > introduced a handful of whitespace problems. I didn't go and track them
> > into the individual patches, but here's the full output from my patch
> > import:
> 
> it didn't add any whitespace problems...
> 
> > 
> > Applying: cleanup: get rid of ASSERT
> > /home/bfoster/repos/xfsdump/.git/rebase-apply/patch:3725: space before tab in indent.
> >         assert( namebuf );
> > /home/bfoster/repos/xfsdump/.git/rebase-apply/patch:5656: trailing whitespace.
> >         assert ( ent != NULL );
> > /home/bfoster/repos/xfsdump/.git/rebase-apply/patch:5855: trailing whitespace.
> >         assert ( ent != NULL );
> 
> s/ASSERT/assert/ does not change any of the whitespace, but it will
> complain about it because the new line has whitespace problems
> because they existed in the old line...
> 

Ok, I saw a few of them highlighted on the console after importing the
patches and for whatever reason, git highlighted the error on line
insertion but not on line removal. That made me think they were
introduced...  disregard.

Brian

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs