On Mon, 2020-01-13 at 08:11 -0800, Bart Van Assche wrote:
> On 1/13/20 7:29 AM, Ewan D. Milne wrote:
> > On Sun, 2020-01-12 at 13:08 -0800, Bart Van Assche wrote:
> > > diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
> > > index c4e087217484..6560908ed50e 100644
> > > --- a/drivers/scsi/qla2xxx/qla_init.c
> > > +++ b/drivers/scsi/qla2xxx/qla_init.c
> > > @@ -4895,6 +4895,8 @@ qla2x00_alloc_fcport(scsi_qla_host_t *vha, gfp_t flags)
> > > void
> > > qla2x00_free_fcport(fc_port_t *fcport)
> > > {
> > > + if (!fcport)
> > > + return;
> > > if (fcport->ct_desc.ct_sns) {
> > > dma_free_coherent(&fcport->vha->hw->pdev->dev,
> > > sizeof(struct ct_sns_pkt), fcport->ct_desc.ct_sns,
> > >
> >
> > I would have fixed this by moving the label to be after the qla2x00_free_fcport()
> > call in qla2x00_configure_local_loop(), which Coverity complained about. And
> > called it something different. (The code could probably be simplified by only
> > making a call to qla2x00_alloc_fcport() in one place, something to think about...)
>
> Hi Ewan,
>
> I have considered the solution that you proposed. The solution shown
> above was chosen because I did not want to introduce any memory leaks in
> qla2x00_configure_local_loop(). There is a "goto cleanup_allocation"
> statement in that function that occurs after the "new_fcport =
> qla2x00_alloc_fcport(vha, GFP_KERNEL)" assignment. That is hard to
> notice because the qla2x00_configure_local_loop() function is so long.
>
Yes, but isn't that after "if (new_fcport == NULL)" where the code has
put the previously allocated fcport into the &vha->vp_fcports list and
was unable to allocate another one?
-Ewan
