From: Brian Foster <bfoster@redhat.com>
To: Dave Chinner <david@fromorbit.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 0/8] xfsdump: Ouchie! My bleeding eyes!
Date: Wed, 28 Oct 2015 07:51:39 -0400 [thread overview]
Message-ID: <20151028115138.GB50552@bfoster.bfoster> (raw)
In-Reply-To: <1444959901-31319-1-git-send-email-david@fromorbit.com>
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:
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 );
warning: 3 lines add whitespace errors.
Applying: build: don't rely on xfs/xfs.h to include necessary headers
Applying: cleanup: kill intgen_t
/home/bfoster/repos/xfsdump/.git/rebase-apply/patch:2018: trailing whitespace.
static int
/home/bfoster/repos/xfsdump/.git/rebase-apply/patch:3295: space before tab in indent.
int fsfd,
/home/bfoster/repos/xfsdump/.git/rebase-apply/patch:6044: trailing whitespace.
int namebuflen;
warning: 3 lines add whitespace errors.
Applying: cleanup: kill u_int*_t types
/home/bfoster/repos/xfsdump/.git/rebase-apply/patch:255: trailing whitespace.
static uint32_t erase_and_verify( drive_t *drivep );
/home/bfoster/repos/xfsdump/.git/rebase-apply/patch:1372: trailing whitespace.
uint s_max_nstreams;/* number of media streams in
/home/bfoster/repos/xfsdump/.git/rebase-apply/patch:1461: trailing whitespace.
DEBUG_displayallsessions( int fd, invt_seshdr_t *hdr, uint ref,
/home/bfoster/repos/xfsdump/.git/rebase-apply/patch:1816: trailing whitespace.
uint16_t d_sum;
warning: 4 lines add whitespace errors.
Applying: cleanup: define a local xfs_ino_t
Applying: cleanup: use system uuid.h headers
Applying: cleanup: move fold_t out of util.h
/home/bfoster/repos/xfsdump/.git/rebase-apply/patch:85: trailing whitespace.
/* flg definitions for preemptchk
warning: 1 line adds whitespace errors.
Applying: cleanup: Kill unnecessary xfs includes
Brian
> -Dave.
>
> _______________________________________________
> 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
next prev parent reply other threads:[~2015-10-28 11:51 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-16 1:44 [PATCH 0/8] xfsdump: Ouchie! My bleeding eyes! Dave Chinner
2015-10-16 1:44 ` [PATCH 1/8] cleanup: get rid of ASSERT Dave Chinner
2015-10-28 11:51 ` Brian Foster
2015-10-28 22:32 ` Dave Chinner
2015-10-29 12:13 ` Brian Foster
2015-10-29 22:26 ` Dave Chinner
2015-10-30 11:39 ` Brian Foster
2015-10-16 1:44 ` [PATCH 2/8] build: don't rely on xfs/xfs.h to include necessary headers Dave Chinner
2015-10-16 1:44 ` [PATCH 3/8] cleanup: kill intgen_t Dave Chinner
2015-10-16 1:44 ` [PATCH 4/8] cleanup: kill u_int*_t types Dave Chinner
2015-10-16 1:44 ` [PATCH 5/8] cleanup: define a local xfs_ino_t Dave Chinner
2015-10-16 1:44 ` [PATCH 6/8] cleanup: use system uuid.h headers Dave Chinner
2015-10-16 1:45 ` [PATCH 7/8] cleanup: move fold_t out of util.h Dave Chinner
2015-10-16 1:45 ` [PATCH 8/8] cleanup: Kill unnecessary xfs includes Dave Chinner
2015-10-28 11:51 ` Brian Foster [this message]
2015-10-28 22:35 ` [PATCH 0/8] xfsdump: Ouchie! My bleeding eyes! Dave Chinner
2015-10-29 12:13 ` Brian Foster
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20151028115138.GB50552@bfoster.bfoster \
--to=bfoster@redhat.com \
--cc=david@fromorbit.com \
--cc=xfs@oss.sgi.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.