[illumos-Developer] Webrev: New ARECA arcmsr driver

Garrett D'Amore garrett at nexenta.com
Wed Mar 23 23:23:06 PDT 2011


On Wed, 2011-03-23 at 23:26 -0400, Albert Lee wrote:
> 2437, 3321: Unnecessary casts.

Fixed.  Those aren't the only ones, but they are the only ones that are
integral types (that I know of.)

> "//" comments... lots of 'em
> , just grep.

Fixed.

> 855-857 is puzzling.

I don't understand what the original code was trying to do, or the
comment which I think was dead wrong.  I think the idea was to be able
to save the original boolean state, but it doesn't actually use any of
these.  I'm going back an revamping this a bit and will send out an
updated review -- largely simplifying the code here to behave more like
other modern HBA drivers.

> 974: Seems like a weird way to say we want to allocate a packet to
> store a single word?

sizeof (void *) is the size of the private field of the packet.  Its
correct.  This code will go away in the future, too, when I make this
code use SCSAv3 DMA handling.

> NumToPtr/PtrToNum could be replaced.

Rewritten as:

#define PtrToNum(p)		(uintptr_t)((void *)p)
#define NumToPtr(ul)		(void *)((uintptr_t)ul)

Stay tuned for an updated webrev.

	- Garrett

> 
> -Albert
> 
> _______________________________________________
> Developer mailing list
> Developer at lists.illumos.org
> http://lists.illumos.org/m/listinfo/developer





More information about the Developer mailing list