xfs
[Top] [All Lists]

Re: review: Simple patch to remove the dmapi support from xfsdump

To: Dean Roehrich <dean.roehrich@xxxxxxx>
Subject: Re: review: Simple patch to remove the dmapi support from xfsdump
From: Russell Cattelan <cattelan@xxxxxxxxxxx>
Date: Mon, 07 Aug 2006 10:30:15 -0500
Cc: Bill Kendall <wkendall@xxxxxxx>, Vlad Apostolov <vapo@xxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <20060807150324.GA8421@xxxxxxxxxxxxxxxxxxxxxxxxxxx>
References: <44D10F9B.8090904@xxxxxxxxxxx> <44D2CA85.3040208@xxxxxxx> <20060804141012.GA26@xxxxxxxxxxxxxxxxxxxxxxxxxxx> <44D36985.1090006@xxxxxxxxxxx> <20060804155850.GA3338@xxxxxxxxxxxxxxxxxxxxxxxxxxx> <44D379A6.9040200@xxxxxxx> <44D38D34.1010503@xxxxxxxxxxx> <44D3C351.7060109@xxxxxxx> <20060807150324.GA8421@xxxxxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Thunderbird 1.5.0.4 (X11/20060614)
Dean Roehrich wrote:
On Fri, Aug 04, 2006 at 04:59:45PM -0500, Bill Kendall wrote:

-#define DMF_EV_BITS    ( (1<<DM_EVENT_DESTROY) | \
-                         (1<<DM_EVENT_READ)    | \
-                         (1<<DM_EVENT_WRITE)   | \
-                         (1<<DM_EVENT_TRUNCATE) )
+#define DMF_EV_BITS    ( (1<<16) | (1<<17) | (1<<18) | (1<<20) )

Don't do that.

Granted, those bits can never be changed else all of your customers will start
a lynch mob and come after you.

At the very least, don't allow those bits to be anonymous--copy that whole
enum from the dmapi header.  Even that I object to, but at least the bits will
_be_ something.
I'll second that.
It seems rather dangerous to have a #define floating around that could
potentially get out of sync with the original, especially if you transport the number and not the enum table. (It make it really hard for cscope to find :-)

Other than that the rest of the patch seem reasonable, it satisfies the goal
of not  requiring libdmapi.

Dean



<Prev in Thread] Current Thread [Next in Thread>