[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