Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unresolved fall-through warnings from GCC 7 #618

Closed
infrastation opened this issue Jul 22, 2017 · 7 comments
Closed

unresolved fall-through warnings from GCC 7 #618

infrastation opened this issue Jul 22, 2017 · 7 comments

Comments

@infrastation
Copy link
Member

A test build on Fedora 26 had produced a number of fall-through warnings, some of which I could resolve but the ones below still stand:

  • One case in print-isoclns.c (not previously detected by Coverity). I could find a few CNLP packet diagrams but it was not enough to understand how to resolve.
./print-isoclns.c: In function ‘clnp_print’:
./print-isoclns.c:1041:16: warning: this statement may fall through [-Wimplicit-fallthrough=]
             if (*(pptr) == NLPID_CLNP) {
                ^
./print-isoclns.c:1048:9: note: here
         case  CLNP_PDU_DT:
         ^~~~
  • Four cases in print-rx.c (existing Coverity defects 603361, 603362, 603363 and 976452). The only description of the Rx protocol used in AFS I could find was the OpenAFS git repository, in which I could not find anything to relate with the case blocks below. My best guess here is all four fall-through cases are not intentional.
./print-rx.c: In function ‘fs_print’:
./print-rx.c:1009:3: warning: this statement may fall through [-Wimplicit-fallthrough=]
   {
   ^
./print-rx.c:1023:3: note: here
   case 65537: /* Fetch data 64 */
   ^~~~
./print-rx.c: In function ‘cb_print’:
./print-rx.c:1250:3: warning: this statement may fall through [-Wimplicit-fallthrough=]
   {
   ^
./print-rx.c:1282:3: note: here
   case 214: {
   ^~~~
In file included from ./print-rx.c:47:0:
./print-rx.c: In function ‘vldb_reply_print’:
./netdissect.h:326:26: warning: this statement may fall through [-Wimplicit-fallthrough=]
 #define ND_PRINT(STUFF) (*ndo->ndo_printf)STUFF
./print-rx.c:777:4: note: in expansion of macro ‘ND_PRINT’
    ND_PRINT((ndo, " %d", _i)); \
    ^~~~~~~~
./print-rx.c:1741:4: note: in expansion of macro ‘INTOUT’
    INTOUT();
    ^~~~~~
./print-rx.c:1742:3: note: here
   case 503: /* Get entry by id */
   ^~~~
In file included from ./print-rx.c:47:0:
./netdissect.h:326:26: warning: this statement may fall through [-Wimplicit-fallthrough=]
 #define ND_PRINT(STUFF) (*ndo->ndo_printf)STUFF
./print-rx.c:777:4: note: in expansion of macro ‘ND_PRINT’
    ND_PRINT((ndo, " %d", _i)); \
    ^~~~~~~~
./print-rx.c:1790:4: note: in expansion of macro ‘INTOUT’
    INTOUT();
    ^~~~~~
./print-rx.c:1791:3: note: here
   case 518: /* Get entry by ID N */
   ^~~~
infrastation added a commit that referenced this issue Jul 26, 2018
This change addresses one of the warnings listed in the bug report.

./print-isoclns.c: In function ‘clnp_print’:
./print-isoclns.c:1054:16: warning: this statement may fall through [-Wimplicit-fallthrough=]
             if (EXTRACT_U_1(pptr) == NLPID_CLNP) {
                ^
./print-isoclns.c:1061:9: note: here
         case  CLNP_PDU_DT:
         ^~~~

[skip ci]
@infrastation
Copy link
Member Author

The CLNP part of this problem has been solved (I have found the complete CLNP spec). I have asked for help with the Rx (AFS3) part on the openafs-devel mailing list.

@kaduk
Copy link

kaduk commented Jul 26, 2018

Hopefully the help offered on the openafs-devel list will come through, but I note that in addition to the "spec" at http://git.openafs.org/?p=openafs.git;a=blob;f=doc/txt/rx-spec.txt;h=c1499132ebba3109231647bf4fbb3c5034a80c55;hb=HEAD and the OpenAFS implementation, the linux AF_RXRPC is the same Rx protocol.

@infrastation
Copy link
Member Author

Thank you for the update. I think I had looked through rx-spec.txt before, but could not make it out if "inline bulk stat" (65536), "callback" (204), "list entry" (510) and "list entry U" (529) relate to the next case block(s) or not. Some of those words do not even appear in the file. I guess those encoding items could be described somewhere in the RPC/XDR-related files within the repository.

@kaduk
Copy link

kaduk commented Jul 27, 2018

I took a look. The first two ("bulk stat" and "callback"; lines 1023 and 1282) should not fall through, but the other two ("list entry" in various forms, lines 1741 and 1790) should fall through.
The answer lies in the RPC-L files at http://git.openafs.org/?p=openafs.git;a=blob;f=src/fsint/afsint.xg;h=9fe4980c6cffcae457789e363393825e29dcb7b1;hb=HEAD#l594, http://git.openafs.org/?p=openafs.git;a=blob;f=src/fsint/afscbint.xg;h=19760bc512c647595662c30e190303b3a20be983;hb=HEAD#l22, and http://git.openafs.org/?p=openafs.git;a=blob;f=src/vlserver/vldbint.xg;h=ff34d12d45454de328439cab3ed896fd564f0c59;hb=HEAD#l341 (also line 396), looking at the appropriate IN or OUT arguments.

@infrastation
Copy link
Member Author

I take your word for it. Now it only remains to add two instances of ND_FALL_THROUGH and two instances of break, possibly with comments, would you like to author this change yourself?

@kaduk
Copy link

kaduk commented Jul 28, 2018

Sadly, I'm a bit overworked at the moment; if you could author the change, that would be great.

infrastation added a commit that referenced this issue Jul 28, 2018
Benjamin Kaduk explains in the issue comments:

The first two ("bulk stat" and "callback"; lines 1023 and 1282) should
not fall through, but the other two ("list entry" in various forms,
lines 1741 and 1790) should fall through. The answer lies in the RPC-L
files at [1], [2] and [3] (also line 396), looking at the appropriate IN
or OUT arguments.

1: http://git.openafs.org/?p=openafs.git;a=blob;f=src/fsint/afsint.xg;h=9fe4980c6cffcae457789e363393825e29dcb7b1;hb=HEAD#l594
2: http://git.openafs.org/?p=openafs.git;a=blob;f=src/fsint/afscbint.xg;h=19760bc512c647595662c30e190303b3a20be983;hb=HEAD#l22
3: http://git.openafs.org/?p=openafs.git;a=blob;f=src/vlserver/vldbint.xg;h=ff34d12d45454de328439cab3ed896fd564f0c59;hb=HEAD#l341
@infrastation infrastation self-assigned this Jul 28, 2018
@infrastation
Copy link
Member Author

The fix is in the master branch now, thank you very much!

mcr pushed a commit that referenced this issue Nov 22, 2018
Benjamin Kaduk explains in the issue comments:

The first two ("bulk stat" and "callback"; lines 1023 and 1282) should
not fall through, but the other two ("list entry" in various forms,
lines 1741 and 1790) should fall through. The answer lies in the RPC-L
files at [1], [2] and [3] (also line 396), looking at the appropriate IN
or OUT arguments.

1: http://git.openafs.org/?p=openafs.git;a=blob;f=src/fsint/afsint.xg;h=9fe4980c6cffcae457789e363393825e29dcb7b1;hb=HEAD#l594
2: http://git.openafs.org/?p=openafs.git;a=blob;f=src/fsint/afscbint.xg;h=19760bc512c647595662c30e190303b3a20be983;hb=HEAD#l22
3: http://git.openafs.org/?p=openafs.git;a=blob;f=src/vlserver/vldbint.xg;h=ff34d12d45454de328439cab3ed896fd564f0c59;hb=HEAD#l341
(backported from commit 8677e5b)
nikhilap1 pushed a commit to nikhilap1/tcpdump that referenced this issue May 10, 2019
This change addresses one of the warnings listed in the bug report.

./print-isoclns.c: In function ‘clnp_print’:
./print-isoclns.c:1054:16: warning: this statement may fall through [-Wimplicit-fallthrough=]
             if (EXTRACT_U_1(pptr) == NLPID_CLNP) {
                ^
./print-isoclns.c:1061:9: note: here
         case  CLNP_PDU_DT:
         ^~~~

[skip ci]
nikhilap1 pushed a commit to nikhilap1/tcpdump that referenced this issue May 10, 2019
Benjamin Kaduk explains in the issue comments:

The first two ("bulk stat" and "callback"; lines 1023 and 1282) should
not fall through, but the other two ("list entry" in various forms,
lines 1741 and 1790) should fall through. The answer lies in the RPC-L
files at [1], [2] and [3] (also line 396), looking at the appropriate IN
or OUT arguments.

1: http://git.openafs.org/?p=openafs.git;a=blob;f=src/fsint/afsint.xg;h=9fe4980c6cffcae457789e363393825e29dcb7b1;hb=HEAD#l594
2: http://git.openafs.org/?p=openafs.git;a=blob;f=src/fsint/afscbint.xg;h=19760bc512c647595662c30e190303b3a20be983;hb=HEAD#l22
3: http://git.openafs.org/?p=openafs.git;a=blob;f=src/vlserver/vldbint.xg;h=ff34d12d45454de328439cab3ed896fd564f0c59;hb=HEAD#l341
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants