On Mon, 2011-10-17 at 09:52 -0700, Roland Dreier wrote: > On Sat, Oct 15, 2011 at 2:57 PM, Madhuranath N Iyengar > <mni@xxxxxxxxxxxxxxxxxxxxx> wrote: > > + if (IS_FWI2_CAPABLE(ha)) > > + iocb = (void *)&imm->imm.imm_ntfy24; > > + else > > + iocb = (void *)&imm->imm.imm_ntfy; > > + > > + qla_tgt_send_notify_ack(vha, iocb, 0, 0, 0, > > + NOTIFY_ACK_SRR_FLAGS_REJECT, > > + NOTIFY_ACK_SRR_REJECT_REASON_UNABLE_TO_PERFORM, > > + NOTIFY_ACK_SRR_FLAGS_REJECT_EXPL_NO_EXPL); > > The code would look even nicer if we could get rid of more of these > "if (IS_FWI2_CAPABLE(ha))" tests. And imm_ntfy24 and imm_ntfy > point to the same memory anyway. So maybe we could add a void * > member to the union (raw_iocb or something?) and not have to worry > about HBA type we're running on? > > - R. Roland, I agree. My goal, with merging was not just to rename (and in the process reduce code), but also eliminate some unneeded constructs like what you talk about above, and maybe simply things a bit. However, I got stuck in some cases, as code is a bit involved, and didn't want to risk breaking something. I can replace the union with a single field in some of those structs. We can then access the field using void *, or using the specific IOCB pointer and eliminate the above macro in some cases. I'll add this to my TODO list. Nic, For all future changes associated with cleanups, I'll submit a newer batch of patches, on top of the earlier batch. The reason is, since these are huge changes, rebasing them becomes a headache sometimes. This way, others can keep reviewing and point to anything they see that needs changes, and I can add them to my todo list, build newer patches, and send them in my next batch. Regards, Madhu -- -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
