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@xxxxxxxxxx>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@xxxxxxxxxxxxx
>
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
|