* [PATCH] xprtrdma: Fix a maybe-uninitialized compiler warning
@ 2021-10-31 13:08 Benjamin Coddington
2021-10-31 15:04 ` Chuck Lever III
0 siblings, 1 reply; 5+ messages in thread
From: Benjamin Coddington @ 2021-10-31 13:08 UTC (permalink / raw)
To: anna.schumaker; +Cc: linux-nfs
This minor fix-up keeps GCC from complaining that "last' may be used
uninitialized", which breaks some build workflows that have been running
with all warnings treated as errors.
Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
net/sunrpc/xprtrdma/frwr_ops.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/sunrpc/xprtrdma/frwr_ops.c b/net/sunrpc/xprtrdma/frwr_ops.c
index f700b34a5bfd..de813fa07daa 100644
--- a/net/sunrpc/xprtrdma/frwr_ops.c
+++ b/net/sunrpc/xprtrdma/frwr_ops.c
@@ -503,7 +503,7 @@ static void frwr_wc_localinv_wake(struct ib_cq *cq, struct ib_wc *wc)
*/
void frwr_unmap_sync(struct rpcrdma_xprt *r_xprt, struct rpcrdma_req *req)
{
- struct ib_send_wr *first, **prev, *last;
+ struct ib_send_wr *first, **prev, *last = NULL;
struct rpcrdma_ep *ep = r_xprt->rx_ep;
const struct ib_send_wr *bad_wr;
struct rpcrdma_mr *mr;
@@ -608,7 +608,7 @@ static void frwr_wc_localinv_done(struct ib_cq *cq, struct ib_wc *wc)
*/
void frwr_unmap_async(struct rpcrdma_xprt *r_xprt, struct rpcrdma_req *req)
{
- struct ib_send_wr *first, *last, **prev;
+ struct ib_send_wr *first, *last = NULL, **prev;
struct rpcrdma_ep *ep = r_xprt->rx_ep;
struct rpcrdma_mr *mr;
int rc;
--
2.31.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] xprtrdma: Fix a maybe-uninitialized compiler warning
2021-10-31 13:08 [PATCH] xprtrdma: Fix a maybe-uninitialized compiler warning Benjamin Coddington
@ 2021-10-31 15:04 ` Chuck Lever III
2021-11-02 16:43 ` Trond Myklebust
0 siblings, 1 reply; 5+ messages in thread
From: Chuck Lever III @ 2021-10-31 15:04 UTC (permalink / raw)
To: Anna Schumaker; +Cc: Linux NFS Mailing List, Benjamin Coddington
> On Oct 31, 2021, at 9:08 AM, Benjamin Coddington <bcodding@redhat.com> wrote:
>
> This minor fix-up keeps GCC from complaining that "last' may be used
> uninitialized", which breaks some build workflows that have been running
> with all warnings treated as errors.
>
> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
Reviewed-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> net/sunrpc/xprtrdma/frwr_ops.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/sunrpc/xprtrdma/frwr_ops.c b/net/sunrpc/xprtrdma/frwr_ops.c
> index f700b34a5bfd..de813fa07daa 100644
> --- a/net/sunrpc/xprtrdma/frwr_ops.c
> +++ b/net/sunrpc/xprtrdma/frwr_ops.c
> @@ -503,7 +503,7 @@ static void frwr_wc_localinv_wake(struct ib_cq *cq, struct ib_wc *wc)
> */
> void frwr_unmap_sync(struct rpcrdma_xprt *r_xprt, struct rpcrdma_req *req)
> {
> - struct ib_send_wr *first, **prev, *last;
> + struct ib_send_wr *first, **prev, *last = NULL;
> struct rpcrdma_ep *ep = r_xprt->rx_ep;
> const struct ib_send_wr *bad_wr;
> struct rpcrdma_mr *mr;
> @@ -608,7 +608,7 @@ static void frwr_wc_localinv_done(struct ib_cq *cq, struct ib_wc *wc)
> */
> void frwr_unmap_async(struct rpcrdma_xprt *r_xprt, struct rpcrdma_req *req)
> {
> - struct ib_send_wr *first, *last, **prev;
> + struct ib_send_wr *first, *last = NULL, **prev;
> struct rpcrdma_ep *ep = r_xprt->rx_ep;
> struct rpcrdma_mr *mr;
> int rc;
> --
> 2.31.1
>
--
Chuck Lever
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] xprtrdma: Fix a maybe-uninitialized compiler warning
2021-10-31 15:04 ` Chuck Lever III
@ 2021-11-02 16:43 ` Trond Myklebust
2021-11-02 16:50 ` Chuck Lever III
0 siblings, 1 reply; 5+ messages in thread
From: Trond Myklebust @ 2021-11-02 16:43 UTC (permalink / raw)
To: anna.schumaker, chuck.lever; +Cc: linux-nfs, bcodding
On Sun, 2021-10-31 at 15:04 +0000, Chuck Lever III wrote:
>
>
> > On Oct 31, 2021, at 9:08 AM, Benjamin Coddington
> > <bcodding@redhat.com> wrote:
> >
> > This minor fix-up keeps GCC from complaining that "last' may be used
> > uninitialized", which breaks some build workflows that have been
> > running
> > with all warnings treated as errors.
> >
> > Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
>
> Reviewed-by: Chuck Lever <chuck.lever@oracle.com>
>
>
> > ---
> > net/sunrpc/xprtrdma/frwr_ops.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/sunrpc/xprtrdma/frwr_ops.c
> > b/net/sunrpc/xprtrdma/frwr_ops.c
> > index f700b34a5bfd..de813fa07daa 100644
> > --- a/net/sunrpc/xprtrdma/frwr_ops.c
> > +++ b/net/sunrpc/xprtrdma/frwr_ops.c
> > @@ -503,7 +503,7 @@ static void frwr_wc_localinv_wake(struct ib_cq
> > *cq, struct ib_wc *wc)
> > */
> > void frwr_unmap_sync(struct rpcrdma_xprt *r_xprt, struct rpcrdma_req
> > *req)
> > {
> > - struct ib_send_wr *first, **prev, *last;
> > + struct ib_send_wr *first, **prev, *last = NULL;
> > struct rpcrdma_ep *ep = r_xprt->rx_ep;
> > const struct ib_send_wr *bad_wr;
> > struct rpcrdma_mr *mr;
Err... Definitely not sufficient.
gcc is absolutely correct to complain here, because if req-
>rl_registered is empty, then the whole rest of the function after that
while() loop is invalid.
> > @@ -608,7 +608,7 @@ static void frwr_wc_localinv_done(struct ib_cq
> > *cq, struct ib_wc *wc)
> > */
> > void frwr_unmap_async(struct rpcrdma_xprt *r_xprt, struct
> > rpcrdma_req *req)
> > {
> > - struct ib_send_wr *first, *last, **prev;
> > + struct ib_send_wr *first, *last = NULL, **prev;
> > struct rpcrdma_ep *ep = r_xprt->rx_ep;
> > struct rpcrdma_mr *mr;
> > int rc;
> > --
> > 2.31.1
> >
Ditto.
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] xprtrdma: Fix a maybe-uninitialized compiler warning
2021-11-02 16:43 ` Trond Myklebust
@ 2021-11-02 16:50 ` Chuck Lever III
2021-11-02 17:36 ` Trond Myklebust
0 siblings, 1 reply; 5+ messages in thread
From: Chuck Lever III @ 2021-11-02 16:50 UTC (permalink / raw)
To: Trond Myklebust; +Cc: Anna Schumaker, Linux NFS Mailing List, bcodding
> On Nov 2, 2021, at 12:43 PM, Trond Myklebust <trondmy@hammerspace.com> wrote:
>
> On Sun, 2021-10-31 at 15:04 +0000, Chuck Lever III wrote:
>>
>>
>>> On Oct 31, 2021, at 9:08 AM, Benjamin Coddington
>>> <bcodding@redhat.com> wrote:
>>>
>>> This minor fix-up keeps GCC from complaining that "last' may be used
>>> uninitialized", which breaks some build workflows that have been
>>> running
>>> with all warnings treated as errors.
>>>
>>> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
>>
>> Reviewed-by: Chuck Lever <chuck.lever@oracle.com>
>>
>>
>>> ---
>>> net/sunrpc/xprtrdma/frwr_ops.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/net/sunrpc/xprtrdma/frwr_ops.c
>>> b/net/sunrpc/xprtrdma/frwr_ops.c
>>> index f700b34a5bfd..de813fa07daa 100644
>>> --- a/net/sunrpc/xprtrdma/frwr_ops.c
>>> +++ b/net/sunrpc/xprtrdma/frwr_ops.c
>>> @@ -503,7 +503,7 @@ static void frwr_wc_localinv_wake(struct ib_cq
>>> *cq, struct ib_wc *wc)
>>> */
>>> void frwr_unmap_sync(struct rpcrdma_xprt *r_xprt, struct rpcrdma_req
>>> *req)
>>> {
>>> - struct ib_send_wr *first, **prev, *last;
>>> + struct ib_send_wr *first, **prev, *last = NULL;
>>> struct rpcrdma_ep *ep = r_xprt->rx_ep;
>>> const struct ib_send_wr *bad_wr;
>>> struct rpcrdma_mr *mr;
>
> Err... Definitely not sufficient.
>
> gcc is absolutely correct to complain here, because if req-
>> rl_registered is empty, then the whole rest of the function after that
> while() loop is invalid.
The callers ensure rl_registered is not empty.
This used to be preferred: don't add code to check conditions
that are known to be true. If that policy is different now,
then yes, this code will have to be restructured.
>>> @@ -608,7 +608,7 @@ static void frwr_wc_localinv_done(struct ib_cq
>>> *cq, struct ib_wc *wc)
>>> */
>>> void frwr_unmap_async(struct rpcrdma_xprt *r_xprt, struct
>>> rpcrdma_req *req)
>>> {
>>> - struct ib_send_wr *first, *last, **prev;
>>> + struct ib_send_wr *first, *last = NULL, **prev;
>>> struct rpcrdma_ep *ep = r_xprt->rx_ep;
>>> struct rpcrdma_mr *mr;
>>> int rc;
>>> --
>>> 2.31.1
>>>
>
> Ditto.
>
> --
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trond.myklebust@hammerspace.com
--
Chuck Lever
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] xprtrdma: Fix a maybe-uninitialized compiler warning
2021-11-02 16:50 ` Chuck Lever III
@ 2021-11-02 17:36 ` Trond Myklebust
0 siblings, 0 replies; 5+ messages in thread
From: Trond Myklebust @ 2021-11-02 17:36 UTC (permalink / raw)
To: chuck.lever; +Cc: linux-nfs, anna.schumaker, bcodding
On Tue, 2021-11-02 at 16:50 +0000, Chuck Lever III wrote:
>
>
> > On Nov 2, 2021, at 12:43 PM, Trond Myklebust
> > <trondmy@hammerspace.com> wrote:
> >
> > On Sun, 2021-10-31 at 15:04 +0000, Chuck Lever III wrote:
> > >
> > >
> > > > On Oct 31, 2021, at 9:08 AM, Benjamin Coddington
> > > > <bcodding@redhat.com> wrote:
> > > >
> > > > This minor fix-up keeps GCC from complaining that "last' may be
> > > > used
> > > > uninitialized", which breaks some build workflows that have
> > > > been
> > > > running
> > > > with all warnings treated as errors.
> > > >
> > > > Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
> > >
> > > Reviewed-by: Chuck Lever <chuck.lever@oracle.com>
> > >
> > >
> > > > ---
> > > > net/sunrpc/xprtrdma/frwr_ops.c | 4 ++--
> > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/net/sunrpc/xprtrdma/frwr_ops.c
> > > > b/net/sunrpc/xprtrdma/frwr_ops.c
> > > > index f700b34a5bfd..de813fa07daa 100644
> > > > --- a/net/sunrpc/xprtrdma/frwr_ops.c
> > > > +++ b/net/sunrpc/xprtrdma/frwr_ops.c
> > > > @@ -503,7 +503,7 @@ static void frwr_wc_localinv_wake(struct
> > > > ib_cq
> > > > *cq, struct ib_wc *wc)
> > > > */
> > > > void frwr_unmap_sync(struct rpcrdma_xprt *r_xprt, struct
> > > > rpcrdma_req
> > > > *req)
> > > > {
> > > > - struct ib_send_wr *first, **prev, *last;
> > > > + struct ib_send_wr *first, **prev, *last = NULL;
> > > > struct rpcrdma_ep *ep = r_xprt->rx_ep;
> > > > const struct ib_send_wr *bad_wr;
> > > > struct rpcrdma_mr *mr;
> >
> > Err... Definitely not sufficient.
> >
> > gcc is absolutely correct to complain here, because if req-
> > > rl_registered is empty, then the whole rest of the function after
> > > that
> > while() loop is invalid.
>
> The callers ensure rl_registered is not empty.
>
> This used to be preferred: don't add code to check conditions
> that are known to be true. If that policy is different now,
> then yes, this code will have to be restructured.
>
If that's the case, then please change those two "while() {}" blocks
into "do { } while();" so that we avoid the apparently unnecessary
first check for whether the list is empty. That would be the real fix
here, and one that clearly documents the expectation.
>
> > > > @@ -608,7 +608,7 @@ static void frwr_wc_localinv_done(struct
> > > > ib_cq
> > > > *cq, struct ib_wc *wc)
> > > > */
> > > > void frwr_unmap_async(struct rpcrdma_xprt *r_xprt, struct
> > > > rpcrdma_req *req)
> > > > {
> > > > - struct ib_send_wr *first, *last, **prev;
> > > > + struct ib_send_wr *first, *last = NULL, **prev;
> > > > struct rpcrdma_ep *ep = r_xprt->rx_ep;
> > > > struct rpcrdma_mr *mr;
> > > > int rc;
> > > > --
> > > > 2.31.1
> > > >
> >
> > Ditto.
> >
> > --
> > Trond Myklebust
> > Linux NFS client maintainer, Hammerspace
> > trond.myklebust@hammerspace.com
>
> --
> Chuck Lever
>
>
>
--
Trond Myklebust
CTO, Hammerspace Inc
4984 El Camino Real, Suite 208
Los Altos, CA 94022
www.hammer.space
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-11-02 17:36 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-31 13:08 [PATCH] xprtrdma: Fix a maybe-uninitialized compiler warning Benjamin Coddington
2021-10-31 15:04 ` Chuck Lever III
2021-11-02 16:43 ` Trond Myklebust
2021-11-02 16:50 ` Chuck Lever III
2021-11-02 17:36 ` Trond Myklebust
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.