| Message ID | 1447220589-9067-5-git-send-email-ira.weiny@intel.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Wed, Nov 11, 2015 at 12:43:05AM -0500, ira.weiny@intel.com wrote: > From: Ira Weiny <ira.weiny@intel.com> > > This goto done is followed by an if (ret) break in the outer switch clause. It > is unnecessary. > > Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com> > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > --- > drivers/staging/rdma/hfi1/diag.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/staging/rdma/hfi1/diag.c b/drivers/staging/rdma/hfi1/diag.c > index 2bb857b2a103..556a47591989 100644 > --- a/drivers/staging/rdma/hfi1/diag.c > +++ b/drivers/staging/rdma/hfi1/diag.c > @@ -1053,7 +1053,6 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg) > break; > default: > ret = -EINVAL; > - goto done; > } > ret = set_link_state(ppd, dev_state); > break; Wut? No it's not. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Nov 11, 2015 at 12:43:05AM -0500, ira.weiny@intel.com wrote: > From: Ira Weiny <ira.weiny@intel.com> > > This goto done is followed by an if (ret) break in the outer switch clause. It > is unnecessary. > > Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com> > Signed-off-by: Ira Weiny <ira.weiny@intel.com> Also these sign offs don't really make sense. Signed-off-by is basically like signing a legal document saying the code in this patch was not stolen from SCO UnixWare. You only need to sign if you have handled the patch. Did Dennis write this patch or did he Ack it or Review it somehow? regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Nov 11, 2015 at 12:03:36PM +0300, Dan Carpenter wrote: >On Wed, Nov 11, 2015 at 12:43:05AM -0500, ira.weiny@intel.com wrote: >> From: Ira Weiny <ira.weiny@intel.com> >> >> This goto done is followed by an if (ret) break in the outer switch clause. It >> is unnecessary. >> >> Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com> >> Signed-off-by: Ira Weiny <ira.weiny@intel.com> > >Also these sign offs don't really make sense. Signed-off-by is >basically like signing a legal document saying the code in this patch >was not stolen from SCO UnixWare. You only need to sign if you have >handled the patch. > >Did Dennis write this patch or did he Ack it or Review it somehow? > >regards, >dan carpenter We both participated in creating the patch(s) in this series. I think the dual signed-off-by is appropriate here. -Denny -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Nov 11, 2015 at 12:01:08PM +0300, Dan Carpenter wrote: > On Wed, Nov 11, 2015 at 12:43:05AM -0500, ira.weiny@intel.com wrote: > > From: Ira Weiny <ira.weiny@intel.com> > > > > This goto done is followed by an if (ret) break in the outer switch clause. It > > is unnecessary. > > > > Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com> > > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > > --- > > drivers/staging/rdma/hfi1/diag.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/drivers/staging/rdma/hfi1/diag.c b/drivers/staging/rdma/hfi1/diag.c > > index 2bb857b2a103..556a47591989 100644 > > --- a/drivers/staging/rdma/hfi1/diag.c > > +++ b/drivers/staging/rdma/hfi1/diag.c > > @@ -1053,7 +1053,6 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg) > > break; > > default: > > ret = -EINVAL; > > - goto done; > > } > > ret = set_link_state(ppd, dev_state); > > break; > > Wut? No it's not. Sorry, you are correct at this point in the series. I got over zealous with my splitting of this patch. This needs to be squashed into staging/rdma/hfi1: Return immediately on error Which returns here. Ira -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/staging/rdma/hfi1/diag.c b/drivers/staging/rdma/hfi1/diag.c index 2bb857b2a103..556a47591989 100644 --- a/drivers/staging/rdma/hfi1/diag.c +++ b/drivers/staging/rdma/hfi1/diag.c @@ -1053,7 +1053,6 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg) break; default: ret = -EINVAL; - goto done; } ret = set_link_state(ppd, dev_state); break; @@ -1201,7 +1200,7 @@ static long hfi1_ioctl(struct file *fp, unsigned int cmd, unsigned long arg) ret = -ENOTTY; break; } -done: + spin_unlock_irqrestore(&dd->hfi1_snoop.snoop_lock, flags); return ret; }