Re: [PATCH 10/30] staging: nvec: Introduce new internal API for msg alloc/free

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

 



On Fri, Sep 23, 2011 at 11:53:58PM +0200, Julian Andres Klode wrote:
> On Fri, Sep 23, 2011 at 10:54:05PM +0300, Dan Carpenter wrote:
> > On Fri, Sep 23, 2011 at 06:38:02PM +0200, Julian Andres Klode wrote:
> > > +static struct nvec_msg *nvec_msg_alloc(struct nvec_chip *nvec)
> > > +{
> > > +	size_t i;
> > This should be "int i;" not "size_t i;"  It's a number between 0 and
> > 64.  Also it would let you avoid the cast below.
> Yes, somehow I'm a size_t person, especially when counting
> array indices. On ARM it won't make a difference technically,
> but on e.g. amd64, the size_t version is supposed to be
> faster (according to some). But if we want to be pedantic,
> we can make that "unsigned int i;"
> 

Please just make it int.  Don't over think things.

Anyway, I have a hard time believing that x86_64 can count to 64
faster in unsigned longs than it can in ints.  I'd have to see some
benchmarks to believe it.  :P  Best to let gcc optimize things
anyway.

> > > +static void nvec_msg_free(struct nvec_chip *nvec, struct nvec_msg *msg)
> > > +{
> > > +	dev_vdbg(nvec->dev, "INFO: Free %i\n", (int) (msg - nvec->msg_pool));
> > 
> > I don't have a cross compile environment set up so I can't compile
> > this, but surely (msg - nvec->msg_pool) generates some kind of
> > compile warning.  I'd think you'd have to cast the struct pointers to
> > unsigned long or something.  I'm not sure also what the printk tells
> > us.
> They are two pointers, you can subtract two pointers and get an
> ptrdiff_t. There is one problem with tx_scratch though, as that's
> not part of the array, it is undefined behavior if called on nvec->tx
> when nvec->tx == nvec->tx_scratch.
> 

Yeah.  You're right.  I misread that.  Sorry.

regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel


[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux