[PATCH] librdmacm: Set errno correctly if status is positive

[PATCH] librdmacm: Set errno correctly if status is positive

[Yuval Shaia]

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-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 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
--- 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;
 }
-- 
2.14.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] librdmacm: Set errno correctly if status is positive

[Jason Gunthorpe]

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-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
>  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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] librdmacm: Set errno correctly if status is positive

[Yuval Shaia]

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-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> >  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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] librdmacm: Set errno correctly if status is positive

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 1822 bytes --]

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-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > >  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.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]