> -----Original Message----- > From: Roel Kluin [mailto:roel.kluin@xxxxxxxxx] > Sent: Monday, June 22, 2009 10:55 AM > To: Abhijeet Joglekar (abjoglek) > Cc: linux-scsi@xxxxxxxxxxxxxxx; Andrew Morton > Subject: [PATCH] remove redundant BUG_ON > > sg_count is unsigned. If negative, a wrap causes the second > BUG_ON to trigger. > > scsi_dma_map may return -ENOMEM, sg_count should be int to catch that. > > Signed-off-by: Roel Kluin <roel.kluin@xxxxxxxxx> > --- > Please use this one instead. > > diff --git a/drivers/scsi/fnic/fnic_scsi.c > b/drivers/scsi/fnic/fnic_scsi.c index eabf365..22a7cd5 100644 > --- a/drivers/scsi/fnic/fnic_scsi.c > +++ b/drivers/scsi/fnic/fnic_scsi.c > @@ -260,7 +260,6 @@ static inline int > fnic_queue_wq_copy_desc(struct fnic *fnic, > char msg[2]; > > if (sg_count) { > - BUG_ON(sg_count < 0); > BUG_ON(sg_count > FNIC_MAX_SG_DESC_CNT); > > /* For each SGE, create a device desc entry */ > @@ -344,7 +343,7 @@ int fnic_queuecommand(struct scsi_cmnd > *sc, void (*done)(struct scsi_cmnd *)) > struct fnic *fnic; > struct vnic_wq_copy *wq; > int ret; > - u32 sg_count; > + int sg_count; > unsigned long flags; > unsigned long ptr; > > Roel, The fix looks good. If dma_map_sg() would have failed, bad things would happen since sg_count was u32. Good catch! It will be better to change the sg_count parameter passed to fnic_queue_wq_copy_desc to int too, so that it is consistent between the 2 functions. Do you want to add that to your patch or you want me to send a patch for that? The BUG_ON(sg_count < 0) can be dropped even though sg_count is an int now, since fnic_queuecommand() checks for sg_count less than zero after call to dma_map_sg. Thanks -- abhijeet -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
