On Thu, Aug 19, 2010 at 2:48 AM, David Dillow <[email protected]> wrote: > On Mon, 2010-08-16 at 20:55 +0200, Bart Van Assche wrote: >> Implements SRP_CRED_REQ and SRP_AER_REQ, which are information units defined >> in the SRP (draft) standard. Adds declarations for the SRP_CRED_REQ, >> SRP_CRED_RSP, SRP_AER_REQ and SRP_AER_RSP information units to >> include/scsi/srp.h. Changes function definition order in ib_srp in order to >> avoid having to add more forward declarations. > > I still don't like the style of this patch -- too much code duplication > and the SRP_TX_IU_* constants are too ugly to live. I'd also prefer to > put the code movement in a separate patch, so in this case I'd accept > the forward declaration and we can do a separate patch later to move the > code around in one swoop. I think there's quite a bit that can > potentially be cleaned up there, and I don't expect you to do it as part > of this series.
I will put the code movement in a separate patch. Regarding code duplication: are you referring to the duplicated test of the SRP response opcode ? If I do not receive any further feedback, I will modify srp_handle_req() such that it accepts a third argument, a function pointer, and will change the call sites of that function such that either a pointer to the srp_handle_cred_req function or a pointer to the srp_handle_aer_req function is passed. Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
