[illumos-Developer] Webrev: New ARECA arcmsr driver

[Garrett D'Amore]

On Thu, 2011-03-24 at 13:41 -0400, Dan McDonald wrote:
> On Thu, Mar 24, 2011 at 09:57:55AM -0700, Garrett D'Amore wrote:
> > All of these changes are small, and hopefully low risk changes.
> > 
> > The new webrev is here: 
> > 
> > http://mexico.purplecow.org/gdamore/webrev/arcmsr-delta/
> 
> Only nit in arcmsr.h:  You nuked this field:
> 
>  854         int                     tgt_scsi_opts[ARCMSR_MAX_TARGETID];
> 
> but I'm not sure why.

No longer used.  It was *never* read from, and written to as part of an
abortive SCSI options (tran_getopt, tran_setopt) handling.  I've cleaned
that code up and no longer need the structure member.

> 
> In arcmsr.c:
> 
> 	- You still have the stupid memcpy(src, dst, 1); calls, even though
>           you fixed stuff around it.

Yeah.  I am not sure I want to bother to fix this.  The same pattern is
repeated several times in the code around the same pass through code,
and the whole thing just needs to be thrown away.  I only fixed the
other stuff around logging because I was purging all cmn_err()'s so I
wanted to get them all to make sure I didn't miss any anywhere else.

The other things I did were some obvious tweaks/cleanups while my cursor
was nearby. 

As it is now, the code is (I think) correct -- its just also horribly
ugly.  Fixing it all the way is more effort than this code is worth.


> 
> Other than that, ship it.

Thanks.

	- Garrett

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