src: fix http2 debug build errors #16432

danbev · 2017-10-24T06:43:39Z

Currently building with debug enabled produces the following errors:

In file included from ../src/node_http2.h:6:
../src/node_http2_core-inl.h:465:18: error: expected ';' after do/while
statement
  CHECK_GT(id, 0)
                 ^
                 ;
../src/node_http2_core-inl.h:469:18: error: use of undeclared identifier
'spec'
  OnPriority(id, spec.stream_id, spec.weight, spec.exclusive);
                 ^
../src/node_http2_core-inl.h:469:34: error: use of undeclared identifier
'spec'
  OnPriority(id, spec.stream_id, spec.weight, spec.exclusive);
                                 ^
../src/node_http2_core-inl.h:469:47: error: use of undeclared identifier
'spec'
  OnPriority(id, spec.stream_id, spec.weight, spec.exclusive);
                                              ^

This commit adds the missing semicolon to fix the above error.

../src/node_http2.cc:92:9: error: reference to non-static member
function must be called; did you mean to call
      it with no arguments?
  CHECK(object->Has(context, env()->ongetpadding_string()).FromJust());
        ^~~~~~
../src/util.h:120:20: note: expanded from macro 'CHECK'
    if (UNLIKELY(!(expr))) {
\
                   ^~~~
../src/util.h:107:44: note: expanded from macro 'UNLIKELY'

For this issue I was not sure what the correct check would be so I've
just commented it out and will update after feedback.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
commit message follows commit guidelines

Affected core subsystem(s)

src

Currently building with debug enabled produces the following errors: In file included from ../src/node_http2.h:6: ../src/node_http2_core-inl.h:465:18: error: expected ';' after do/while statement CHECK_GT(id, 0) ^ ; ../src/node_http2_core-inl.h:469:18: error: use of undeclared identifier 'spec' OnPriority(id, spec.stream_id, spec.weight, spec.exclusive); ^ ../src/node_http2_core-inl.h:469:34: error: use of undeclared identifier 'spec' OnPriority(id, spec.stream_id, spec.weight, spec.exclusive); ^ ../src/node_http2_core-inl.h:469:47: error: use of undeclared identifier 'spec' OnPriority(id, spec.stream_id, spec.weight, spec.exclusive); ^ This commit adds the missing semicolon to fix the above error. ../src/node_http2.cc:92:9: error: reference to non-static member function must be called; did you mean to call it with no arguments? CHECK(object->Has(context, env()->ongetpadding_string()).FromJust()); ^~~~~~ ../src/util.h:120:20: note: expanded from macro 'CHECK' if (UNLIKELY(!(expr))) { \ ^~~~ ../src/util.h:107:44: note: expanded from macro 'UNLIKELY' For this issue I was not sure what the correct check would be so I've just commented it out and will update after feedback.

gireeshpunathil · 2017-10-24T07:22:35Z

src/node_http2.cc

@@ -89,7 +89,7 @@ ssize_t Http2Session::OnCallbackPadding(size_t frameLen,
  Context::Scope context_scope(context);

 #if defined(DEBUG) && DEBUG
-  CHECK(object->Has(context, env()->ongetpadding_string()).FromJust());
+  // CHECK(object->Has(context, env()->ongetpadding_string()).FromJust());


I think
CHECK(object->Has(context, env()->ongetpadding_string().FromJust()));
is the right one. Without which, the whole expression was evaluated to be a value type, while we expect to have a boolean type for the CHECK.

apapirovski

LGTM with the comment addressed

apapirovski · 2017-10-24T08:03:52Z

src/node_http2.cc

@@ -89,7 +89,7 @@ ssize_t Http2Session::OnCallbackPadding(size_t frameLen,
  Context::Scope context_scope(context);

 #if defined(DEBUG) && DEBUG
-  CHECK(object->Has(context, env()->ongetpadding_string()).FromJust());
+  // CHECK(object->Has(context, env()->ongetpadding_string()).FromJust());


This should be

CHECK(object()->Has(context, env()->ongetpadding_string()).FromJust());

Thanks, I was missing where object was coming from. Will update shortly.

apapirovski · 2017-10-24T16:09:54Z

I would like to see this land in less than 48 hours, pending a CI run, if no one objects. Feel free to use the 'confused' emoji to let me know otherwise :)

addaleax · 2017-10-24T22:15:37Z

Landed in a9f5084, thanks!

Currently building with debug enabled produces the following errors: In file included from ../src/node_http2.h:6: ../src/node_http2_core-inl.h:465:18: error: expected ';' after do/while statement CHECK_GT(id, 0) ^ ; ../src/node_http2_core-inl.h:469:18: error: use of undeclared identifier 'spec' OnPriority(id, spec.stream_id, spec.weight, spec.exclusive); ^ ../src/node_http2_core-inl.h:469:34: error: use of undeclared identifier 'spec' OnPriority(id, spec.stream_id, spec.weight, spec.exclusive); ^ ../src/node_http2_core-inl.h:469:47: error: use of undeclared identifier 'spec' OnPriority(id, spec.stream_id, spec.weight, spec.exclusive); ^ This commit adds the missing semicolon to fix the above error. ../src/node_http2.cc:92:9: error: reference to non-static member function must be called; did you mean to call it with no arguments? CHECK(object->Has(context, env()->ongetpadding_string()).FromJust()); ^~~~~~ ../src/util.h:120:20: note: expanded from macro 'CHECK' if (UNLIKELY(!(expr))) { \ ^~~~ ../src/util.h:107:44: note: expanded from macro 'UNLIKELY' For this issue I was not sure what the correct check would be so I've just commented it out and will update after feedback. PR-URL: #16432 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>

Currently building with debug enabled produces the following errors: In file included from ../src/node_http2.h:6: ../src/node_http2_core-inl.h:465:18: error: expected ';' after do/while statement CHECK_GT(id, 0) ^ ; ../src/node_http2_core-inl.h:469:18: error: use of undeclared identifier 'spec' OnPriority(id, spec.stream_id, spec.weight, spec.exclusive); ^ ../src/node_http2_core-inl.h:469:34: error: use of undeclared identifier 'spec' OnPriority(id, spec.stream_id, spec.weight, spec.exclusive); ^ ../src/node_http2_core-inl.h:469:47: error: use of undeclared identifier 'spec' OnPriority(id, spec.stream_id, spec.weight, spec.exclusive); ^ This commit adds the missing semicolon to fix the above error. ../src/node_http2.cc:92:9: error: reference to non-static member function must be called; did you mean to call it with no arguments? CHECK(object->Has(context, env()->ongetpadding_string()).FromJust()); ^~~~~~ ../src/util.h:120:20: note: expanded from macro 'CHECK' if (UNLIKELY(!(expr))) { \ ^~~~ ../src/util.h:107:44: note: expanded from macro 'UNLIKELY' For this issue I was not sure what the correct check would be so I've just commented it out and will update after feedback. PR-URL: nodejs/node#16432 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>

Currently building with debug enabled produces the following errors: In file included from ../src/node_http2.h:6: ../src/node_http2_core-inl.h:465:18: error: expected ';' after do/while statement CHECK_GT(id, 0) ^ ; ../src/node_http2_core-inl.h:469:18: error: use of undeclared identifier 'spec' OnPriority(id, spec.stream_id, spec.weight, spec.exclusive); ^ ../src/node_http2_core-inl.h:469:34: error: use of undeclared identifier 'spec' OnPriority(id, spec.stream_id, spec.weight, spec.exclusive); ^ ../src/node_http2_core-inl.h:469:47: error: use of undeclared identifier 'spec' OnPriority(id, spec.stream_id, spec.weight, spec.exclusive); ^ This commit adds the missing semicolon to fix the above error. ../src/node_http2.cc:92:9: error: reference to non-static member function must be called; did you mean to call it with no arguments? CHECK(object->Has(context, env()->ongetpadding_string()).FromJust()); ^~~~~~ ../src/util.h:120:20: note: expanded from macro 'CHECK' if (UNLIKELY(!(expr))) { \ ^~~~ ../src/util.h:107:44: note: expanded from macro 'UNLIKELY' For this issue I was not sure what the correct check would be so I've just commented it out and will update after feedback. PR-URL: nodejs#16432 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>

Currently building with debug enabled produces the following errors: In file included from ../src/node_http2.h:6: ../src/node_http2_core-inl.h:465:18: error: expected ';' after do/while statement CHECK_GT(id, 0) ^ ; ../src/node_http2_core-inl.h:469:18: error: use of undeclared identifier 'spec' OnPriority(id, spec.stream_id, spec.weight, spec.exclusive); ^ ../src/node_http2_core-inl.h:469:34: error: use of undeclared identifier 'spec' OnPriority(id, spec.stream_id, spec.weight, spec.exclusive); ^ ../src/node_http2_core-inl.h:469:47: error: use of undeclared identifier 'spec' OnPriority(id, spec.stream_id, spec.weight, spec.exclusive); ^ This commit adds the missing semicolon to fix the above error. ../src/node_http2.cc:92:9: error: reference to non-static member function must be called; did you mean to call it with no arguments? CHECK(object->Has(context, env()->ongetpadding_string()).FromJust()); ^~~~~~ ../src/util.h:120:20: note: expanded from macro 'CHECK' if (UNLIKELY(!(expr))) { \ ^~~~ ../src/util.h:107:44: note: expanded from macro 'UNLIKELY' For this issue I was not sure what the correct check would be so I've just commented it out and will update after feedback. PR-URL: nodejs/node#16432 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels Oct 24, 2017

danbev added the http2 Issues or PRs related to the http2 subsystem. label Oct 24, 2017

danbev force-pushed the http2_core_inl-debug-build branch from 368192d to 06726f2 Compare October 24, 2017 06:56

danbev changed the title ~~src: add missing semicolon to debug statement~~ src: fix http2 debug build errors Oct 24, 2017

gireeshpunathil reviewed Oct 24, 2017

View reviewed changes

apapirovski approved these changes Oct 24, 2017

View reviewed changes

squash: uncomment check

c5ec2eb

bnoordhuis approved these changes Oct 24, 2017

View reviewed changes

cjihrig approved these changes Oct 24, 2017

View reviewed changes

danbev mentioned this pull request Oct 24, 2017

node build failing in debug mode #16440

Closed

jasnell approved these changes Oct 24, 2017

View reviewed changes

tniessen approved these changes Oct 24, 2017

View reviewed changes

addaleax approved these changes Oct 24, 2017

View reviewed changes

addaleax closed this Oct 24, 2017

apapirovski mentioned this pull request Oct 26, 2017

Add missing semicolon #16517

Closed

2 tasks

jasnell mentioned this pull request Oct 30, 2017

Proposal: 2017-10-31, Version 9.0.0 (Current) #15135

Merged

Fishrock123 mentioned this pull request Oct 30, 2017

Node.js 8.9.0 Proposal? #16615

Closed

gibfahn mentioned this pull request Oct 31, 2017

v8.9.0 proposal #16630

Merged

rvagg mentioned this pull request Nov 1, 2017

Additional builders: shared libraries and debug nodejs/build#108

Closed

danbev deleted the http2_core_inl-debug-build branch November 16, 2017 08:39

MylesBorins added the dont-land-on-v6.x label Nov 16, 2017

leonardoadame mentioned this pull request May 13, 2024

[Snyk] Security upgrade eslint from 8.22.0 to 8.26.0 leonardoadame/node#818

Open

leonardoadame mentioned this pull request May 14, 2024

[Snyk] Fix for 2 vulnerabilities leonardoadame/node#839

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

src: fix http2 debug build errors #16432

src: fix http2 debug build errors #16432

danbev commented Oct 24, 2017

gireeshpunathil Oct 24, 2017

apapirovski left a comment

apapirovski Oct 24, 2017

danbev Oct 24, 2017

apapirovski commented Oct 24, 2017 •

edited

Loading

addaleax commented Oct 24, 2017

src: fix http2 debug build errors #16432

src: fix http2 debug build errors #16432

Conversation

danbev commented Oct 24, 2017

Checklist

Affected core subsystem(s)

gireeshpunathil Oct 24, 2017

Choose a reason for hiding this comment

apapirovski left a comment

Choose a reason for hiding this comment

apapirovski Oct 24, 2017

Choose a reason for hiding this comment

danbev Oct 24, 2017

Choose a reason for hiding this comment

apapirovski commented Oct 24, 2017 • edited Loading

addaleax commented Oct 24, 2017

apapirovski commented Oct 24, 2017 •

edited

Loading