public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Russell Cattelan <cattelan@thebarn.com>
To: Dean Roehrich <dean.roehrich@sun.com>
Cc: Bill Kendall <wkendall@sgi.com>, Vlad Apostolov <vapo@sgi.com>,
	xfs@oss.sgi.com
Subject: Re: review: Simple patch to remove the dmapi support from xfsdump
Date: Mon, 07 Aug 2006 10:30:15 -0500	[thread overview]
Message-ID: <44D75C87.8050402@thebarn.com> (raw)
In-Reply-To: <20060807150324.GA8421@kickball-mn.Central.Sun.COM>

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
>
>   

  reply	other threads:[~2006-08-07 15:30 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-08-02 20:48 review: Simple patch to remove the dmapi support from xfsdump Russell Cattelan
2006-08-04  4:18 ` Vlad Apostolov
2006-08-04 14:10   ` Dean Roehrich
2006-08-04 15:36     ` Russell Cattelan
2006-08-04 15:58       ` Dean Roehrich
2006-08-04 16:45         ` Bill Kendall
2006-08-04 17:08           ` Dean Roehrich
2006-08-04 17:33           ` Eric Sandeen
2006-08-04 18:08           ` Russell Cattelan
2006-08-04 21:59             ` Bill Kendall
2006-08-07 15:03               ` Dean Roehrich
2006-08-07 15:30                 ` Russell Cattelan [this message]
2006-08-07 15:52                   ` Chris Wedgwood
2006-08-07 16:51                     ` Dean Roehrich
2006-08-07 19:13                       ` Bill Kendall
2006-08-07 19:25                         ` Dean Roehrich
2006-08-07  0:00     ` Vlad Apostolov
2006-08-04 14:24 ` Bill Kendall

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=44D75C87.8050402@thebarn.com \
    --to=cattelan@thebarn.com \
    --cc=dean.roehrich@sun.com \
    --cc=vapo@sgi.com \
    --cc=wkendall@sgi.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox