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

deps: suppress zlib compiler warnings #40343

Closed
wants to merge 2 commits into from
Closed

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Oct 6, 2021

Currently, there are a number of compilation warnings from zlib like the
following one:

../deps/zlib/infback.c: In function ‘inflateBack’:
../deps/zlib/infback.c:479:25: warning:
this statement may fall through [-Wimplicit-fallthrough=]
  479 |             state->mode = LEN;
      |             ~~~~~~~~~~~~^~~~~
../deps/zlib/infback.c:481:9: note: here
  481 |         case LEN:
      |         ^~~~

In this case there is no break statement and the intention is to fall
through:

           Tracev((stderr, "inflate:       codes ok\n"));
           state->mode = LEN;

        case LEN:

This commit adds -Wno-implicit-fallthrough to zlib.gyp to suppress
these warnings.

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. zlib Issues and PRs related to the zlib subsystem. labels Oct 6, 2021
@nodejs-github-bot
Copy link
Collaborator

Currently, there are a number of compilation warnings from zlib like the
following one:

../deps/zlib/infback.c: In function ‘inflateBack’:
../deps/zlib/infback.c:479:25: warning:
this statement may fall through [-Wimplicit-fallthrough=]
  479 |             state->mode = LEN;
      |             ~~~~~~~~~~~~^~~~~
../deps/zlib/infback.c:481:9: note: here
  481 |         case LEN:
      |         ^~~~

In this case there is no break statement and the intention is to fall
through:

           Tracev((stderr, "inflate:       codes ok\n"));
           state->mode = LEN;

        case LEN:

This commit adds -Wno-implicit-fallthrough' to zlib.gyp to suppress
these warnings.
Move cflags into conditions section and only enable for non-windows
architectures.
@nodejs-github-bot
Copy link
Collaborator

@danbev
Copy link
Contributor Author

danbev commented Oct 11, 2021

Re-run of failing /node-test-binary-windows-js-suites ✔️

danbev added a commit that referenced this pull request Oct 11, 2021
Currently, there are a number of compilation warnings from zlib like the
following one:

../deps/zlib/infback.c: In function ‘inflateBack’:
../deps/zlib/infback.c:479:25: warning:
this statement may fall through [-Wimplicit-fallthrough=]
  479 |             state->mode = LEN;
      |             ~~~~~~~~~~~~^~~~~
../deps/zlib/infback.c:481:9: note: here
  481 |         case LEN:
      |         ^~~~

In this case there is no break statement and the intention is to fall
through:

           Tracev((stderr, "inflate:       codes ok\n"));
           state->mode = LEN;

        case LEN:

This commit adds -Wno-implicit-fallthrough' to zlib.gyp to suppress
these warnings.

PR-URL: #40343
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@danbev
Copy link
Contributor Author

danbev commented Oct 11, 2021

Landed in 5d7bd86.

@danbev danbev closed this Oct 11, 2021
targos pushed a commit that referenced this pull request Oct 13, 2021
Currently, there are a number of compilation warnings from zlib like the
following one:

../deps/zlib/infback.c: In function ‘inflateBack’:
../deps/zlib/infback.c:479:25: warning:
this statement may fall through [-Wimplicit-fallthrough=]
  479 |             state->mode = LEN;
      |             ~~~~~~~~~~~~^~~~~
../deps/zlib/infback.c:481:9: note: here
  481 |         case LEN:
      |         ^~~~

In this case there is no break statement and the intention is to fall
through:

           Tracev((stderr, "inflate:       codes ok\n"));
           state->mode = LEN;

        case LEN:

This commit adds -Wno-implicit-fallthrough' to zlib.gyp to suppress
these warnings.

PR-URL: #40343
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@richardlau richardlau mentioned this pull request Oct 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ci PRs that need a full CI run. zlib Issues and PRs related to the zlib subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants