From: Brian Foster <bfoster@redhat.com>
To: Dave Chinner <david@fromorbit.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 1/8] cleanup: get rid of ASSERT
Date: Fri, 30 Oct 2015 07:39:51 -0400 [thread overview]
Message-ID: <20151030113950.GA54905@bfoster.bfoster> (raw)
In-Reply-To: <20151029222609.GC10656@dastard>
On Fri, Oct 30, 2015 at 09:26:09AM +1100, Dave Chinner wrote:
> On Thu, Oct 29, 2015 at 08:13:34AM -0400, Brian Foster wrote:
...
> > Then why not try to undef/redef in xfsdump or just rename the #define
> > that's used? I don't care too much either way, I just don't follow why
> > there's a need to change behavior at all to fix a naming conflict.
> >
> > Are we saying that ASSERT() probably shouldn't exist in userspace (incl.
> > xfsprogs) and we should always use the generic assert() mechanism? Or
> > are we saying ASSERT() can exist in userspace, but it's purely a libxfs
> > thing and should not be exported beyond that (e.g., libxfs can use
> > ASSERT(), actual userspace tools like repair, etc. should eventually use
> > assert())..?
>
> Precisely this. ASSERT is used in the kernel side libxfs code, and
> so only exists in userspace to support the libxfs code in userspace.
> We don't want the libxfs code in userspace aborting on corrupt
> structures or invalid situations, because we need to
> handle/parse/repair such brokenness. IOWS, the ASSERTs in libxfs/ in
> userspace need to be turned off for userspace to work correctly.
> That's what the DEBUG define in libxfs does.
>
Indeed.
> That leads to the xfsprogs userspace still needing an assert
> facility - that comes from using assert() and NDEBUG to turn it off
> and on.
>
> Essentially, ASSERT is internal to libxfs and should not be exported
> anywhere. assert() should used outside libxfs in xfsprogs and other
> XFS code bases.
>
Makes sense to me. Thanks for the explanation. It would be good if the
commit log description included some of this info. Otherwise, for the
series:
Acked-by: Brian Foster <bfoster@redhat.com>
> 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
next prev parent reply other threads:[~2015-10-30 11:39 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 [this message]
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 ` [PATCH 0/8] xfsdump: Ouchie! My bleeding eyes! Brian Foster
2015-10-28 22:35 ` 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=20151030113950.GA54905@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.