Re: [PATCH 05/37] ata: use get/put_endian helpers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 29 May 2008 22:26:18 -0400 Mark Lord <liml@xxxxxx> wrote:

> Harvey Harrison wrote:
> > Signed-off-by: Harvey Harrison <harvey.harrison@xxxxxxxxx>
> > ---
> >  drivers/ata/pdc_adma.c   |   12 ++++++------
> >  drivers/ata/sata_qstor.c |   12 +++++-------
> >  2 files changed, 11 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/ata/pdc_adma.c b/drivers/ata/pdc_adma.c
> > index be53545..162eecb 100644
> > --- a/drivers/ata/pdc_adma.c
> > +++ b/drivers/ata/pdc_adma.c
> > @@ -284,11 +284,11 @@ static int adma_fill_sg(struct ata_queued_cmd *qc)
> >  		u32 len;
> >  
> >  		addr = (u32)sg_dma_address(sg);
> > -		*(__le32 *)(buf + i) = cpu_to_le32(addr);
> > +		put_le32(addr, (__le32 *)(buf + i));
> ..
> 
> I don't get it.
> 
> What's the point here?

Readability and maintainability.  Once one becomes familar with a
particular idion like this you can read it and say "ah, I know what
that's doing" rather than having to peer at every character and work it
out at each site where it happens.

As usual: a PITA now, but better long-term.

otoh,

- I think the args are backwards

- I don't like the use of the put_*() namespace.  It makes it look
  like a uaccess operation.
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux