xfs
[Top] [All Lists]

Re: [PATCH 08/12] xfs: Remove the macro XFS_BUF_SET_PTR

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 08/12] xfs: Remove the macro XFS_BUF_SET_PTR
From: Alex Elder <aelder@xxxxxxx>
Date: Mon, 25 Jul 2011 10:57:36 -0500
Cc: Chandra Seetharaman <sekharan@xxxxxxxxxx>, <xfs@xxxxxxxxxxx>
In-reply-to: <20110724113543.GB26332@xxxxxxxxxxxxx>
References: <20110722003226.21069.58401.sendpatchset@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> <20110722003333.21069.32295.sendpatchset@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> <1311363510.2771.103.camel@doink> <20110724113543.GB26332@xxxxxxxxxxxxx>
Reply-to: <aelder@xxxxxxx>
On Sun, 2011-07-24 at 07:35 -0400, Christoph Hellwig wrote:
> On Fri, Jul 22, 2011 at 02:38:30PM -0500, Alex Elder wrote:
> > > -                                     (__psint_t)count), split);
> > > +         xfs_buf_associate_memory(bp,
> > > +                         (xfs_caddr_t)((__psint_t)&(iclog->ic_header)+
> > > +                             (__psint_t)count), split);
> > 
> > If you just cast the address appropriately you can make this
> > look a lot nicer:
> > 
> >             xfs_buf_associate_memory(bp,
> >                     (void *) &iclog->ic_header + count, split);
> 
> If we have to cast anyway I'd suggest casting to char * at least, as
> that is standard behaviour. I'm okay with using the gcc void pointer
> arithmetics extension when it saves us ugliness, but in this case it
> doesn't really buy us anything.

Yes, I agree.  I guess I had the "void *" addition on the brain
when I did this.  Chandra, I would like to make this small change
before I commit this.  OK with you?

> Btw, nice follow-on cleanups would be to kill off the xfs_caddr_t and
> __psint_t/__psunsigned_t types entirely.

Yes.  What do you suggest to use for a pointer-sized
type?  Just cast to (long) and rely on the usual arithmetic
conversions to do the right thing?

                                        -Alex


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