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

fallthrough warnings in nanomsg #215

Closed
nickdesaulniers opened this issue Jul 1, 2019 · 1 comment · Fixed by #217
Closed

fallthrough warnings in nanomsg #215

nickdesaulniers opened this issue Jul 1, 2019 · 1 comment · Fixed by #217
Assignees

Comments

@nickdesaulniers
Copy link
Owner

probably because my system is using a newer compiler, but npm i I'm seeing:

In file included from ../deps/nanomsg/src/transports/ipc/sipc.c:26:0:
../deps/nanomsg/src/transports/ipc/sipc.c: In function ‘nn_sipc_handler’:
../deps/nanomsg/src/transports/ipc/../../utils/err.h:48:12: warning: this statement may fall through [-Wimplicit-fallthrough=]
         if (nn_slow (!(x))) {\
            ^
../deps/nanomsg/src/transports/ipc/sipc.c:377:21: note: in expansion of macro ‘nn_assert’
                     nn_assert (0);
                     ^~~~~~~~~
../deps/nanomsg/src/transports/ipc/sipc.c:380:13: note: here
             case NN_USOCK_SHUTDOWN:
             ^~~~

In file included from ../deps/nanomsg/src/transports/utils/streamhdr.c:28:0:
../deps/nanomsg/src/transports/utils/streamhdr.c: In function ‘nn_streamhdr_handler’:
../deps/nanomsg/src/transports/utils/../../utils/err.h:48:12: warning: this statement may fall through [-Wimplicit-fallthrough=]
         if (nn_slow (!(x))) {\
            ^
../deps/nanomsg/src/transports/utils/streamhdr.c:239:17: note: in expansion of macro ‘nn_assert’
                 nn_assert (0);
                 ^~~~~~~~~
../deps/nanomsg/src/transports/utils/streamhdr.c:242:9: note: here
         case NN_STREAMHDR_SRC_TIMER:
         ^~~~

I suspect this has already been reported and fixed upstream, in which case we need to up-rev our dependency. If it's not reported/fixed upstream, should be straightforward to do so.

@nickdesaulniers
Copy link
Owner Author

or we could disable them in https://github.com/nickdesaulniers/node-nanomsg/blob/master/deps/linux.gypi. Probably should reaudit those flags.

@nickdesaulniers nickdesaulniers self-assigned this Jul 1, 2019
nickdesaulniers added a commit that referenced this issue Jul 1, 2019
I suspect these were copied from w/e Makefile upstream nanomsg used at
the time.  Looks like they've since moved to CMake and now just set
-Wall -Wextra.

Remove old flags we don't need anymore, and add new ones to silence the
warnings in our current snapshot of nanomsg.

These flags should be audited and potentially removed (or added) next
time we uprev our snapshot of nanomsg.

Fixes #215
nickdesaulniers added a commit that referenced this issue Jul 2, 2019
I suspect these were copied from w/e Makefile upstream nanomsg used at
the time.  Looks like they've since moved to CMake and now just set
-Wall -Wextra.

Remove old flags we don't need anymore, and add new ones to silence the
warnings in our current snapshot of nanomsg.

These flags should be audited and potentially removed (or added) next
time we uprev our snapshot of nanomsg.

Fixes #215
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant