Re: [scsi-qedf] question about parameter ordering
On Wed, 3 May 2017, 2:36pm, Randy Dunlap wrote: > On 05/03/17 11:29, Chad Dupuis wrote: > > > > On Wed, 3 May 2017, 1:58pm, Gustavo A. R. Silva wrote: > > > >> > >> Hello everybody, > >> > >> While looking into Coverity ID 1402011 I ran into the following piece of > >> code > >> at drivers/scsi/qedf/qedf_io.c:2057: > >> > >> /* Fill FC header */ > >> fc_hdr = &(tm_req->req_fc_hdr); > >> sid = fcport->sid; > >> did = fcport->rdata->ids.port_id; > >> __fc_fill_fc_hdr(fc_hdr, FC_RCTL_DD_UNSOL_CMD, sid, did, > >> FC_TYPE_FCP, FC_FC_FIRST_SEQ | FC_FC_END_SEQ | > >> FC_FC_SEQ_INIT, 0); > >> > >> The issue here is that the position of arguments in the call to > >> __fc_fill_fc_hdr() function do not match the ordering of the parameters: > >> > >> _sid_ is passed to _did_ > >> _did_ is passed to _sid_ > >> > >> this is the function prototype: > >> > >> static inline void __fc_fill_fc_hdr(struct fc_frame_header *fh, > >>enum fc_rctl r_ctl, > >>u32 did, u32 sid, enum fc_fh_type type, > >>u32 f_ctl, u32 parm_offset) > >> > >> My question here is if this is intentionala? > > > > This may have been but this code has been superseded by commit > > be086e7c53f1fac51eed14523b28f2214b548dd2.B > > what git tree is that in? > > Thanks. > Apologies, a couple of stray characters got into the commit ID. This is in mainline: commit be086e7c53f1fac51eed14523b28f2214b548dd2 Author: Mintz, YuvalDate: Sat Mar 11 18:39:18 2017 +0200 qed*: Utilize Firmware 8.15.3.0 > >> In case it is not, I will send a patch to fix it. But first it would be > >> great > >> to hear any comment about it. > >> > >> By the way... the same is happening at drivers/scsi/qedf/qedf_els.c:109 > > > > May be a bug here so you could send a patch. > > >
Re: [scsi-qedf] question about parameter ordering
On 05/03/17 11:29, Chad Dupuis wrote: > > On Wed, 3 May 2017, 1:58pm, Gustavo A. R. Silva wrote: > >> >> Hello everybody, >> >> While looking into Coverity ID 1402011 I ran into the following piece of code >> at drivers/scsi/qedf/qedf_io.c:2057: >> >> /* Fill FC header */ >> fc_hdr = &(tm_req->req_fc_hdr); >> sid = fcport->sid; >> did = fcport->rdata->ids.port_id; >> __fc_fill_fc_hdr(fc_hdr, FC_RCTL_DD_UNSOL_CMD, sid, did, >> FC_TYPE_FCP, FC_FC_FIRST_SEQ | FC_FC_END_SEQ | >> FC_FC_SEQ_INIT, 0); >> >> The issue here is that the position of arguments in the call to >> __fc_fill_fc_hdr() function do not match the ordering of the parameters: >> >> _sid_ is passed to _did_ >> _did_ is passed to _sid_ >> >> this is the function prototype: >> >> static inline void __fc_fill_fc_hdr(struct fc_frame_header *fh, >>enum fc_rctl r_ctl, >>u32 did, u32 sid, enum fc_fh_type type, >>u32 f_ctl, u32 parm_offset) >> >> My question here is if this is intentionala? > > This may have been but this code has been superseded by commit > be086e7c53f1fac51eed14523b28f2214b548dd2.B what git tree is that in? Thanks. >> In case it is not, I will send a patch to fix it. But first it would be great >> to hear any comment about it. >> >> By the way... the same is happening at drivers/scsi/qedf/qedf_els.c:109 > > May be a bug here so you could send a patch. -- ~Randy
Re: [scsi-qedf] question about parameter ordering
Hi Chad, Quoting Chad Dupuis: On Wed, 3 May 2017, 1:58pm, Gustavo A. R. Silva wrote: Hello everybody, While looking into Coverity ID 1402011 I ran into the following piece of code at drivers/scsi/qedf/qedf_io.c:2057: /* Fill FC header */ fc_hdr = &(tm_req->req_fc_hdr); sid = fcport->sid; did = fcport->rdata->ids.port_id; __fc_fill_fc_hdr(fc_hdr, FC_RCTL_DD_UNSOL_CMD, sid, did, FC_TYPE_FCP, FC_FC_FIRST_SEQ | FC_FC_END_SEQ | FC_FC_SEQ_INIT, 0); The issue here is that the position of arguments in the call to __fc_fill_fc_hdr() function do not match the ordering of the parameters: _sid_ is passed to _did_ _did_ is passed to _sid_ this is the function prototype: static inline void __fc_fill_fc_hdr(struct fc_frame_header *fh, enum fc_rctl r_ctl, u32 did, u32 sid, enum fc_fh_type type, u32 f_ctl, u32 parm_offset) My question here is if this is intentionala? This may have been but this code has been superseded by commit be086e7c53f1fac51eed14523b28f2214b548dd2.B Oh OK, great. In case it is not, I will send a patch to fix it. But first it would be great to hear any comment about it. By the way... the same is happening at drivers/scsi/qedf/qedf_els.c:109 May be a bug here so you could send a patch. I'll send a patch for this shortly. Thanks for your comments. -- Gustavo A. R. Silva
Re: [scsi-qedf] question about parameter ordering
On Wed, 3 May 2017, 1:58pm, Gustavo A. R. Silva wrote: > > Hello everybody, > > While looking into Coverity ID 1402011 I ran into the following piece of code > at drivers/scsi/qedf/qedf_io.c:2057: > > /* Fill FC header */ > fc_hdr = &(tm_req->req_fc_hdr); > sid = fcport->sid; > did = fcport->rdata->ids.port_id; > __fc_fill_fc_hdr(fc_hdr, FC_RCTL_DD_UNSOL_CMD, sid, did, > FC_TYPE_FCP, FC_FC_FIRST_SEQ | FC_FC_END_SEQ | > FC_FC_SEQ_INIT, 0); > > The issue here is that the position of arguments in the call to > __fc_fill_fc_hdr() function do not match the ordering of the parameters: > > _sid_ is passed to _did_ > _did_ is passed to _sid_ > > this is the function prototype: > > static inline void __fc_fill_fc_hdr(struct fc_frame_header *fh, >enum fc_rctl r_ctl, >u32 did, u32 sid, enum fc_fh_type type, >u32 f_ctl, u32 parm_offset) > > My question here is if this is intentionala? This may have been but this code has been superseded by commit be086e7c53f1fac51eed14523b28f2214b548dd2.B > > In case it is not, I will send a patch to fix it. But first it would be great > to hear any comment about it. > > By the way... the same is happening at drivers/scsi/qedf/qedf_els.c:109 May be a bug here so you could send a patch. > > Thank you > -- > Gustavo A. R. Silva > > > >
[scsi-qedf] question about parameter ordering
Hello everybody, While looking into Coverity ID 1402011 I ran into the following piece of code at drivers/scsi/qedf/qedf_io.c:2057: /* Fill FC header */ fc_hdr = &(tm_req->req_fc_hdr); sid = fcport->sid; did = fcport->rdata->ids.port_id; __fc_fill_fc_hdr(fc_hdr, FC_RCTL_DD_UNSOL_CMD, sid, did, FC_TYPE_FCP, FC_FC_FIRST_SEQ | FC_FC_END_SEQ | FC_FC_SEQ_INIT, 0); The issue here is that the position of arguments in the call to __fc_fill_fc_hdr() function do not match the ordering of the parameters: _sid_ is passed to _did_ _did_ is passed to _sid_ this is the function prototype: static inline void __fc_fill_fc_hdr(struct fc_frame_header *fh, enum fc_rctl r_ctl, u32 did, u32 sid, enum fc_fh_type type, u32 f_ctl, u32 parm_offset) My question here is if this is intentional? In case it is not, I will send a patch to fix it. But first it would be great to hear any comment about it. By the way... the same is happening at drivers/scsi/qedf/qedf_els.c:109 Thank you -- Gustavo A. R. Silva