[bug report] i40iw: add main, hdr, status

[bug report] i40iw: add main, hdr, status

[Dan Carpenter]

Hello Faisal Latif,

The patch 8e06af711bf2: "i40iw: add main, hdr, status" from Jan 20,
2016, leads to the following static checker warning:

	drivers/infiniband/hw/i40iw/i40iw_main.c:606 i40iw_create_cqp()
	error: uninitialized symbol 'maj_err'.

drivers/infiniband/hw/i40iw/i40iw_main.c
   599          status = dev->cqp_ops->cqp_init(dev->cqp, &cqp_init_info);
   600          if (status) {
   601                  i40iw_pr_err("cqp init status %d\n", status);
   602                  goto exit;
   603          }
   604          status = dev->cqp_ops->cqp_create(dev->cqp, &maj_err, &min_err);
   605          if (status) {
   606                  i40iw_pr_err("cqp create status %d maj_err %d min_err %d\n",
   607                               status, maj_err, min_err);

Not all error paths in i40iw_sc_cqp_create() initialized maj_err and
min_err.   Can't we just delete this printk?

   608                  goto exit;
   609          }

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [bug report] i40iw: add main, hdr, status

[Shiraz Saleem]

On Tue, Jan 17, 2017 at 04:58:50PM +0300, Dan Carpenter wrote:
> Hello Faisal Latif,
> 
> The patch 8e06af711bf2: "i40iw: add main, hdr, status" from Jan 20,
> 2016, leads to the following static checker warning:
> 
> 	drivers/infiniband/hw/i40iw/i40iw_main.c:606 i40iw_create_cqp()
> 	error: uninitialized symbol 'maj_err'.
> 
> drivers/infiniband/hw/i40iw/i40iw_main.c
>    599          status = dev->cqp_ops->cqp_init(dev->cqp, &cqp_init_info);
>    600          if (status) {
>    601                  i40iw_pr_err("cqp init status %d\n", status);
>    602                  goto exit;
>    603          }
>    604          status = dev->cqp_ops->cqp_create(dev->cqp, &maj_err, &min_err);
>    605          if (status) {
>    606                  i40iw_pr_err("cqp create status %d maj_err %d min_err %d\n",
>    607                               status, maj_err, min_err);
> 
> Not all error paths in i40iw_sc_cqp_create() initialized maj_err and
> min_err.   Can't we just delete this printk?

We would like to keep maj_err and min_err prints  

In i40iw_create_cqp, can we initialize the locals maj_err and min_err to 
0 instead of deleting the printk. 

> 
>    608                  goto exit;
>    609          }
> 
> regards,
> dan carpenter
> --
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [bug report] i40iw: add main, hdr, status

[Yuval Shaia]

On Tue, Jan 17, 2017 at 12:52:48PM -0600, Shiraz Saleem wrote:
> On Tue, Jan 17, 2017 at 04:58:50PM +0300, Dan Carpenter wrote:
> > Hello Faisal Latif,
> > 
> > The patch 8e06af711bf2: "i40iw: add main, hdr, status" from Jan 20,
> > 2016, leads to the following static checker warning:
> > 
> > 	drivers/infiniband/hw/i40iw/i40iw_main.c:606 i40iw_create_cqp()
> > 	error: uninitialized symbol 'maj_err'.
> > 
> > drivers/infiniband/hw/i40iw/i40iw_main.c
> >    599          status = dev->cqp_ops->cqp_init(dev->cqp, &cqp_init_info);
> >    600          if (status) {
> >    601                  i40iw_pr_err("cqp init status %d\n", status);
> >    602                  goto exit;
> >    603          }
> >    604          status = dev->cqp_ops->cqp_create(dev->cqp, &maj_err, &min_err);
> >    605          if (status) {
> >    606                  i40iw_pr_err("cqp create status %d maj_err %d min_err %d\n",
> >    607                               status, maj_err, min_err);
> > 
> > Not all error paths in i40iw_sc_cqp_create() initialized maj_err and
> > min_err.   Can't we just delete this printk?
> 
> We would like to keep maj_err and min_err prints  
> 
> In i40iw_create_cqp, can we initialize the locals maj_err and min_err to 
> 0 instead of deleting the printk. 

Don't you think it is the job of cqp_create to set maj_err and min_err when
replying with an error?

> 
> > 
> >    608                  goto exit;
> >    609          }
> > 
> > regards,
> > dan carpenter
> > --
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [bug report] i40iw: add main, hdr, status

[Saleem, Shiraz]

> On Tue, Jan 17, 2017 at 12:52:48PM -0600, Shiraz Saleem wrote:
> > On Tue, Jan 17, 2017 at 04:58:50PM +0300, Dan Carpenter wrote:
> > > Hello Faisal Latif,
> > >
> > > The patch 8e06af711bf2: "i40iw: add main, hdr, status" from Jan 20,
> > > 2016, leads to the following static checker warning:
> > >
> > > 	drivers/infiniband/hw/i40iw/i40iw_main.c:606 i40iw_create_cqp()
> > > 	error: uninitialized symbol 'maj_err'.
> > >
> > > drivers/infiniband/hw/i40iw/i40iw_main.c
> > >    599          status = dev->cqp_ops->cqp_init(dev->cqp, &cqp_init_info);
> > >    600          if (status) {
> > >    601                  i40iw_pr_err("cqp init status %d\n", status);
> > >    602                  goto exit;
> > >    603          }
> > >    604          status = dev->cqp_ops->cqp_create(dev->cqp, &maj_err,
> &min_err);
> > >    605          if (status) {
> > >    606                  i40iw_pr_err("cqp create status %d maj_err %d min_err
> %d\n",
> > >    607                               status, maj_err, min_err);
> > >
> > > Not all error paths in i40iw_sc_cqp_create() initialized maj_err and
> > > min_err.   Can't we just delete this printk?
> >
> > We would like to keep maj_err and min_err prints
> >
> > In i40iw_create_cqp, can we initialize the locals maj_err and min_err
> > to
> > 0 instead of deleting the printk.
> 
> Don't you think it is the job of cqp_create to set maj_err and min_err when replying
> with an error?
> 
Agreed. This is a better approach. I sent out a patch for the same. 

Thank you Dan and Yuval.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html