[PATCH rdma-core] bnxt_re: Fix -Wimplicit-fallthrough warnings in GCC 7.1

[PATCH rdma-core] bnxt_re: Fix -Wimplicit-fallthrough warnings in GCC 7.1

[Leon Romanovsky]

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

[59/189] Building C object providers/bnxt_re/CMakeFiles/bnxt_re-rdmav2.dir/verbs.c.o
../providers/bnxt_re/verbs.c: In function ‘bnxt_re_post_send’:
../providers/bnxt_re/verbs.c:1222:18: warning: this statement may fall through [-Wimplicit-fallthrough=]
    hdr->key_immd = htole32(be32toh(wr->imm_data));
                  ^
../providers/bnxt_re/verbs.c:1223:3: note: here
   case IBV_WR_SEND:
   ^~~~
../providers/bnxt_re/verbs.c:1232:18: warning: this statement may fall through [-Wimplicit-fallthrough=]
    hdr->key_immd = htole32(be32toh(wr->imm_data));
                  ^
../providers/bnxt_re/verbs.c:1233:3: note: here
   case IBV_WR_RDMA_WRITE:

Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 providers/bnxt_re/verbs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/providers/bnxt_re/verbs.c b/providers/bnxt_re/verbs.c
index 1fe79671..da0fe733 100644
--- a/providers/bnxt_re/verbs.c
+++ b/providers/bnxt_re/verbs.c
@@ -1220,6 +1220,7 @@ int bnxt_re_post_send(struct ibv_qp *ibvqp, struct ibv_send_wr *wr,
 			 * LE platform be32toh will do the job.
 			 */
 			hdr->key_immd = htole32(be32toh(wr->imm_data));
+			/* FALLTHROUGH */
 		case IBV_WR_SEND:
 			if (qp->qptyp == IBV_QPT_UD)
 				bytes = bnxt_re_build_ud_sqe(qp, sqe, wr,
@@ -1230,6 +1231,7 @@ int bnxt_re_post_send(struct ibv_qp *ibvqp, struct ibv_send_wr *wr,
 			break;
 		case IBV_WR_RDMA_WRITE_WITH_IMM:
 			hdr->key_immd = htole32(be32toh(wr->imm_data));
+			/* FALLTHROUGH */
 		case IBV_WR_RDMA_WRITE:
 			bytes = bnxt_re_build_rdma_sqe(qp, sqe, wr, is_inline);
 			break;
-- 
2.13.2

--
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 rdma-core] bnxt_re: Fix -Wimplicit-fallthrough warnings in GCC 7.1

[Bart Van Assche]

On Thu, 2017-07-13 at 16:39 +0300, Leon Romanovsky wrote:
> diff --git a/providers/bnxt_re/verbs.c b/providers/bnxt_re/verbs.c
> index 1fe79671..da0fe733 100644
> --- a/providers/bnxt_re/verbs.c
> +++ b/providers/bnxt_re/verbs.c
> @@ -1220,6 +1220,7 @@ int bnxt_re_post_send(struct ibv_qp *ibvqp, struct ibv_send_wr *wr,
>  			 * LE platform be32toh will do the job.
>  			 */
>  			hdr->key_immd = htole32(be32toh(wr->imm_data));
> +			/* FALLTHROUGH */

Hello Leon,

In other rdma-core source files the annotation "/* fall through */" is used.
See also the output of git grep -nHi '/\* *fall[ -]*through'. Should we ensure
that these annotations are consistent in the rdma-core source tree?

Bart.--
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 rdma-core] bnxt_re: Fix -Wimplicit-fallthrough warnings in GCC 7.1

[Leon Romanovsky]

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

On Thu, Jul 13, 2017 at 03:06:11PM +0000, Bart Van Assche wrote:
> On Thu, 2017-07-13 at 16:39 +0300, Leon Romanovsky wrote:
> > diff --git a/providers/bnxt_re/verbs.c b/providers/bnxt_re/verbs.c
> > index 1fe79671..da0fe733 100644
> > --- a/providers/bnxt_re/verbs.c
> > +++ b/providers/bnxt_re/verbs.c
> > @@ -1220,6 +1220,7 @@ int bnxt_re_post_send(struct ibv_qp *ibvqp, struct ibv_send_wr *wr,
> >  			 * LE platform be32toh will do the job.
> >  			 */
> >  			hdr->key_immd = htole32(be32toh(wr->imm_data));
> > +			/* FALLTHROUGH */
>
> Hello Leon,
>
> In other rdma-core source files the annotation "/* fall through */" is used.
> See also the output of git grep -nHi '/\* *fall[ -]*through'. Should we ensure
> that these annotations are consistent in the rdma-core source tree?

Yes, better to have consistency. I took this "FALLTHROUGH" from kernel
source.

I'll resend.

Thanks

>
> Bart.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH rdma-core] bnxt_re: Fix -Wimplicit-fallthrough warnings in GCC 7.1

[Jason Gunthorpe]

On Thu, Jul 13, 2017 at 04:39:35PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> [59/189] Building C object providers/bnxt_re/CMakeFiles/bnxt_re-rdmav2.dir/verbs.c.o
> ../providers/bnxt_re/verbs.c: In function ‘bnxt_re_post_send’:
> ../providers/bnxt_re/verbs.c:1222:18: warning: this statement may fall through [-Wimplicit-fallthrough=]
>     hdr->key_immd = htole32(be32toh(wr->imm_data));
>                   ^
> ../providers/bnxt_re/verbs.c:1223:3: note: here
>    case IBV_WR_SEND:
>    ^~~~
> ../providers/bnxt_re/verbs.c:1232:18: warning: this statement may fall through [-Wimplicit-fallthrough=]
>     hdr->key_immd = htole32(be32toh(wr->imm_data));
>                   ^
> ../providers/bnxt_re/verbs.c:1233:3: note: here
>    case IBV_WR_RDMA_WRITE:


Can you follow the same approach as I used in

commit 47cd2efcd4cd416ead0a1a48153aa27a2e178ce5
Author: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Date:   Tue Jan 31 14:55:50 2017 -0700

    Annotate correct switch fall throughs with SWITCH_FALLTHROUGH

Or convert those cases to this approach?

>  			 * LE platform be32toh will do the job.
>  			 */
>  			hdr->key_immd = htole32(be32toh(wr->imm_data));
> +			/* FALLTHROUGH */

Use the SWITCH_FALLTHROUGH; macro

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 rdma-core] bnxt_re: Fix -Wimplicit-fallthrough warnings in GCC 7.1

[Bart Van Assche]

On Thu, 2017-07-13 at 11:51 -0600, Jason Gunthorpe wrote:
> Can you follow the same approach as I used in
> 
> commit 47cd2efcd4cd416ead0a1a48153aa27a2e178ce5
> Author: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> Date:   Tue Jan 31 14:55:50 2017 -0700
> 
>     Annotate correct switch fall throughs with SWITCH_FALLTHROUGH
> 
> Or convert those cases to this approach?
> 
> >  			 * LE platform be32toh will do the job.
> >  			 */
> >  			hdr->key_immd = htole32(be32toh(wr->imm_data));
> > +			/* FALLTHROUGH */
> 
> Use the SWITCH_FALLTHROUGH; macro

Hello Jason,

Both Coverity and gcc 7 understand fall-through comments. Do we really need
a macro to annotate fall-through in switch statements?

Bart.--
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 rdma-core] bnxt_re: Fix -Wimplicit-fallthrough warnings in GCC 7.1

[Jason Gunthorpe]

On Thu, Jul 13, 2017 at 05:54:12PM +0000, Bart Van Assche wrote:
> On Thu, 2017-07-13 at 11:51 -0600, Jason Gunthorpe wrote:
> > Can you follow the same approach as I used in
> > 
> > commit 47cd2efcd4cd416ead0a1a48153aa27a2e178ce5
> > Author: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> > Date:   Tue Jan 31 14:55:50 2017 -0700
> > 
> >     Annotate correct switch fall throughs with SWITCH_FALLTHROUGH
> > 
> > Or convert those cases to this approach?
> > 
> > >  			 * LE platform be32toh will do the job.
> > >  			 */
> > >  			hdr->key_immd = htole32(be32toh(wr->imm_data));
> > > +			/* FALLTHROUGH */
> > 
> > Use the SWITCH_FALLTHROUGH; macro
> 
> Hello Jason,
> 
> Both Coverity and gcc 7 understand fall-through comments. Do we
> really need a macro to annotate fall-through in switch statements?

Probably not, but I added it a while ago already based on some other
documentation..

The advantage of the macro is that all compilers verify it is spelled
correctly, while it is easy to make a mistake with comments..

In any case, let us do it consistently throughtout..

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 rdma-core] bnxt_re: Fix -Wimplicit-fallthrough warnings in GCC 7.1

[Leon Romanovsky]

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

On Thu, Jul 13, 2017 at 12:15:36PM -0600, Jason Gunthorpe wrote:
> On Thu, Jul 13, 2017 at 05:54:12PM +0000, Bart Van Assche wrote:
> > On Thu, 2017-07-13 at 11:51 -0600, Jason Gunthorpe wrote:
> > > Can you follow the same approach as I used in
> > >
> > > commit 47cd2efcd4cd416ead0a1a48153aa27a2e178ce5
> > > Author: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> > > Date:   Tue Jan 31 14:55:50 2017 -0700
> > >
> > >     Annotate correct switch fall throughs with SWITCH_FALLTHROUGH
> > >
> > > Or convert those cases to this approach?
> > >
> > > >  			 * LE platform be32toh will do the job.
> > > >  			 */
> > > >  			hdr->key_immd = htole32(be32toh(wr->imm_data));
> > > > +			/* FALLTHROUGH */
> > >
> > > Use the SWITCH_FALLTHROUGH; macro
> >
> > Hello Jason,
> >
> > Both Coverity and gcc 7 understand fall-through comments. Do we
> > really need a macro to annotate fall-through in switch statements?
>
> Probably not, but I added it a while ago already based on some other
> documentation..
>
> The advantage of the macro is that all compilers verify it is spelled
> correctly, while it is easy to make a mistake with comments..
>
> In any case, let us do it consistently throughtout..

No problem, it is less than 1 minute change.

The thing which bothers me, why didn't we see this warning in Travis CI?
We need to treat all compilation warnings as errors.

Thanks

>
> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH rdma-core] bnxt_re: Fix -Wimplicit-fallthrough warnings in GCC 7.1

[Jason Gunthorpe]

On Thu, Jul 13, 2017 at 10:08:29PM +0300, Leon Romanovsky wrote:

> The thing which bothers me, why didn't we see this warning in Travis CI?
> We need to treat all compilation warnings as errors.

Travis does not have gcc 7 right now, the 6.x compiler does not
produce the warning by default.

I haven't looked lately if we can switch travis to 7 or not.

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