re: [SCSI] bfa: Implement LUN Masking feature using the SCSI Slave Callouts.

re: [SCSI] bfa: Implement LUN Masking feature using the SCSI Slave Callouts.

[Dan Carpenter]

Hello Krishna Gudipati,

This is a semi-automatic email about new static checker warnings.

The patch 5b7db7af522d: "[SCSI] bfa: Implement LUN Masking feature 
using the SCSI Slave Callouts." from Dec 20, 2011, leads to the 
following Smatch complaint:

drivers/scsi/bfa/bfad_im.c +962 bfad_im_slave_alloc()
	 warn: variable dereferenced before check 'rport' (see line 959)

drivers/scsi/bfa/bfad_im.c
   957          struct fc_rport *rport = starget_to_rport(scsi_target(sdev));
   958		struct bfad_itnim_data_s *itnim_data =
   959					(struct bfad_itnim_data_s *) rport->dd_data;
                                                                     ^^^^^^^
New dereference.

   960		struct bfa_s *bfa = itnim_data->itnim->bfa_itnim->bfa;
   961	
   962		if (!rport || fc_remote_port_chkready(rport))
                     ^^^^^
Old check.

   963			return -ENXIO;
   964	

regards,
dan carpenter

Re: [Ksummit-2012-discuss] [ATTEND] Your upstream maintainer just isn't that into you...

[Dan Carpenter]

On Mon, Jun 25, 2012 at 06:44:56PM -0700, Dan Williams wrote:
> Can we do a better job of bounding the maximum latency for acceptance
> or rejection of a patch?

I sent quite a few patches all over the kernel as part of my Smatch
work.  If I don't get feedback on my patch then I assume it will be
merged.  I've gone through my sent-mail box and collected the
patches which weren't merged.  Hopefully, this is useful in
discussing maintainer submitter feedback.

Some of these patches are probably wrong but it would have been nice
to get some feedback on that.

The SCSI subsystem obviously stands out as pretty bad.  It's the
only subsystem where you can send a patch over and over and not get
a response.  It's not that I think all my patches should be merged
without any review and free hugs for everyone...  For all the other
patches in this list, I feel that the patches was dropped by
mistake but in SCSI it was silently ignored as a policy.

Causes of patches being lost:
* Poor changelog on a cleanup patch.
* I didn't CC the person who introduced the bug so I never got
  their acked-by.
* The patch wasn't obvious and the maintainer was planning to review
  it later and forgot.
* Confusion about who was supposed to pull a patch.
* Poor communication between the maintainer and me and I didn't
  realize they wanted me to redo the patch.  Don't phrase your
  statements as questions.

regards,
dan carpenter

Re: [SCSI] bfa: Implement LUN Masking feature using the SCSI Slave Callouts.

[Dan Carpenter]

Hi,

This bug is still present in linux-next.

regards,
dan carpenter

On Wed, Jan 11, 2012 at 12:32:34PM +0300, Dan Carpenter wrote:
> Hello Krishna Gudipati,
> 
> This is a semi-automatic email about new static checker warnings.
> 
> The patch 5b7db7af522d: "[SCSI] bfa: Implement LUN Masking feature 
> using the SCSI Slave Callouts." from Dec 20, 2011, leads to the 
> following Smatch complaint:
> 
> drivers/scsi/bfa/bfad_im.c +962 bfad_im_slave_alloc()
> 	 warn: variable dereferenced before check 'rport' (see line 959)
> 
> drivers/scsi/bfa/bfad_im.c
>    957          struct fc_rport *rport = starget_to_rport(scsi_target(sdev));
>    958		struct bfad_itnim_data_s *itnim_data =
>    959					(struct bfad_itnim_data_s *) rport->dd_data;
>                                                                      ^^^^^^^
> New dereference.
> 
>    960		struct bfa_s *bfa = itnim_data->itnim->bfa_itnim->bfa;
>    961	
>    962		if (!rport || fc_remote_port_chkready(rport))
>                      ^^^^^
> Old check.
> 
>    963			return -ENXIO;
>    964	
> 
> regards,
> dan carpenter
>