Skip to content

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Oct 31, 2018

This commit updates the check of the offset argument passed to
CallJSOnreadMethod to avoid doing a cast as this currently generates
the following compiler warning when doing a debug build:

./src/stream_base.cc:295:3:
warning: comparison of integers of different signs:
'int32_t' (aka 'int') and 'size_t' (aka 'unsigned long')
[-Wsign-compare]
  CHECK_EQ(static_cast<int32_t>(offset), offset);
  ^        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~  ~~~~~~
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Oct 31, 2018
@addaleax
Copy link
Member

(Fwiw, we usually keep stream: as a subsystem for the userland/JS streams module… I’ve been using src: for all or most StreamBase changes)

This commit updates the check of the offset argument passed to
CallJSOnreadMethod to avoid doing a cast as this currently generates
the following compiler warning when doing a debug build:

./src/stream_base.cc:295:3:
warning: comparison of integers of different signs:
'int32_t' (aka 'int') and 'size_t' (aka 'unsigned long')
[-Wsign-compare]
  CHECK_EQ(static_cast<int32_t>(offset), offset);
  ^        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~  ~~~~~~
@danbev danbev force-pushed the stream_base_debug_warning branch from 0fce52b to 62a3402 Compare November 1, 2018 05:16
@danbev danbev changed the title stream: fix compiler warning for debug build src: fix compiler warning for debug build Nov 1, 2018
@danbev
Copy link
Contributor Author

danbev commented Nov 1, 2018

Trott pushed a commit to Trott/io.js that referenced this pull request Nov 3, 2018
This commit updates the check of the offset argument passed to
CallJSOnreadMethod to avoid doing a cast as this currently generates
the following compiler warning when doing a debug build:

./src/stream_base.cc:295:3:
warning: comparison of integers of different signs:
'int32_t' (aka 'int') and 'size_t' (aka 'unsigned long')
[-Wsign-compare]
  CHECK_EQ(static_cast<int32_t>(offset), offset);
  ^        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~  ~~~~~~

PR-URL: nodejs#23994
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@Trott
Copy link
Member

Trott commented Nov 3, 2018

Landed in 2ea70ea

@Trott Trott closed this Nov 3, 2018
targos pushed a commit that referenced this pull request Nov 3, 2018
This commit updates the check of the offset argument passed to
CallJSOnreadMethod to avoid doing a cast as this currently generates
the following compiler warning when doing a debug build:

./src/stream_base.cc:295:3:
warning: comparison of integers of different signs:
'int32_t' (aka 'int') and 'size_t' (aka 'unsigned long')
[-Wsign-compare]
  CHECK_EQ(static_cast<int32_t>(offset), offset);
  ^        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~  ~~~~~~

PR-URL: #23994
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@danbev danbev deleted the stream_base_debug_warning branch November 5, 2018 05:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants