| Message ID | 20180110160721.14469-1-yuval.shaia@oracle.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Leon Romanovsky |
| Headers | show |
On Wed, Jan 10, 2018 at 06:07:21PM +0200, Yuval Shaia wrote: > No need to convert to positive if status is already positive. > > Fixes: 1ef5c3a84 ("librdmacm: Set errno correctly in ucma_complete") > > Signed-off-by: Yuval Shaia <yuval.shaia@oracle.com> > librdmacm/cma.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/librdmacm/cma.c b/librdmacm/cma.c > index 25ebaaee..fb2dc5e4 100644 > +++ b/librdmacm/cma.c > @@ -866,7 +866,7 @@ int ucma_complete(struct rdma_cm_id *id) > else if (id_priv->id.event->status < 0) > ret = ERR(-id_priv->id.event->status); > else > - ret = ERR(-id_priv->id.event->status); > + ret = ERR(id_priv->id.event->status); This code hurts my brain - why is status sometimes a possitive errno and sometimes a negative errno? Jason -- 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, Jan 10, 2018 at 11:29:32AM -0700, Jason Gunthorpe wrote: > On Wed, Jan 10, 2018 at 06:07:21PM +0200, Yuval Shaia wrote: > > No need to convert to positive if status is already positive. > > > > Fixes: 1ef5c3a84 ("librdmacm: Set errno correctly in ucma_complete") > > > > Signed-off-by: Yuval Shaia <yuval.shaia@oracle.com> > > librdmacm/cma.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/librdmacm/cma.c b/librdmacm/cma.c > > index 25ebaaee..fb2dc5e4 100644 > > +++ b/librdmacm/cma.c > > @@ -866,7 +866,7 @@ int ucma_complete(struct rdma_cm_id *id) > > else if (id_priv->id.event->status < 0) > > ret = ERR(-id_priv->id.event->status); > > else > > - ret = ERR(-id_priv->id.event->status); > > + ret = ERR(id_priv->id.event->status); > > This code hurts my brain - why is status sometimes a possitive errno > and sometimes a negative errno? Have no idea :) But since current code ask "if (id_priv->id.event->status < 0)" i assume it can. > > Jason -- 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, 2018-01-10 at 20:33 +0200, Yuval Shaia wrote: > On Wed, Jan 10, 2018 at 11:29:32AM -0700, Jason Gunthorpe wrote: > > On Wed, Jan 10, 2018 at 06:07:21PM +0200, Yuval Shaia wrote: > > > No need to convert to positive if status is already positive. > > > > > > Fixes: 1ef5c3a84 ("librdmacm: Set errno correctly in ucma_complete") > > > > > > Signed-off-by: Yuval Shaia <yuval.shaia@oracle.com> > > > librdmacm/cma.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/librdmacm/cma.c b/librdmacm/cma.c > > > index 25ebaaee..fb2dc5e4 100644 > > > +++ b/librdmacm/cma.c > > > @@ -866,7 +866,7 @@ int ucma_complete(struct rdma_cm_id *id) > > > else if (id_priv->id.event->status < 0) > > > ret = ERR(-id_priv->id.event->status); > > > else > > > - ret = ERR(-id_priv->id.event->status); > > > + ret = ERR(id_priv->id.event->status); > > > > This code hurts my brain - why is status sometimes a possitive errno > > and sometimes a negative errno? > > Have no idea :) > But since current code ask "if (id_priv->id.event->status < 0)" i assume it > can. Since this item covers the entire gamut of events, I wouldn't be surprised if there are some events defined with negative returns and some with positive returns. Anyway, an audit of all the returns is more than I want to do just for this patch (and I did a quick look through in the kernel to see if the answer to this is easy to get to and unfortunately it's not because of how many places you need to track down the treatment of the status item), so I'll apply it as is.
diff --git a/librdmacm/cma.c b/librdmacm/cma.c index 25ebaaee..fb2dc5e4 100644 --- a/librdmacm/cma.c +++ b/librdmacm/cma.c @@ -866,7 +866,7 @@ int ucma_complete(struct rdma_cm_id *id) else if (id_priv->id.event->status < 0) ret = ERR(-id_priv->id.event->status); else - ret = ERR(-id_priv->id.event->status); + ret = ERR(id_priv->id.event->status); } return ret; }
No need to convert to positive if status is already positive. Fixes: 1ef5c3a84 ("librdmacm: Set errno correctly in ucma_complete") Signed-off-by: Yuval Shaia <yuval.shaia@oracle.com> --- librdmacm/cma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)