nghttp3_conv: fix call to undeclared functions 'ntohl' and 'htons'

[MatteoBax]

I don't think it's the right method but this solves these errors when compiling for Android arm64:

../deps/ngtcp2/nghttp3/lib/nghttp3_conv.c:49:21: error: call to undeclared function 'ntohs'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    return (int64_t)ntohs(n.n16);
                    ^
../deps/ngtcp2/nghttp3/lib/nghttp3_conv.c:53:21: error: call to undeclared function 'ntohl'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    return (int64_t)ntohl(n.n32);
                    ^
../deps/ngtcp2/nghttp3/lib/nghttp3_conv.c:69:7: error: call to undeclared function 'ntohl'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
  n = nghttp3_htonl64(n);
      ^
../deps/ngtcp2/nghttp3/lib/nghttp3_conv.h:73:32: note: expanded from macro 'nghttp3_htonl64'
#    define nghttp3_htonl64(N) nghttp3_bswap64(N)
                               ^
../deps/ngtcp2/nghttp3/lib/nghttp3_conv.h:60:17: note: expanded from macro 'nghttp3_bswap64'
    ((uint64_t)(ntohl((uint32_t)(N))) << 32 | ntohl((uint32_t)((N) >> 32)))
                ^
../deps/ngtcp2/nghttp3/lib/nghttp3_conv.c:74:7: error: call to undeclared function 'ntohl'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
  n = nghttp3_htonl64(n);
      ^
../deps/ngtcp2/nghttp3/lib/nghttp3_conv.h:73:32: note: expanded from macro 'nghttp3_htonl64'
#    define nghttp3_htonl64(N) nghttp3_bswap64(N)
                               ^
../deps/ngtcp2/nghttp3/lib/nghttp3_conv.h:60:17: note: expanded from macro 'nghttp3_bswap64'
    ((uint64_t)(ntohl((uint32_t)(N))) << 32 | ntohl((uint32_t)((N) >> 32)))
                ^
../deps/ngtcp2/nghttp3/lib/nghttp3_conv.c:79:7: error: call to undeclared function 'htonl'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
  n = htonl(n);
      ^
../deps/ngtcp2/nghttp3/lib/nghttp3_conv.c:84:7: error: call to undeclared function 'htonl'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
  n = htonl(n);
      ^
../deps/ngtcp2/nghttp3/lib/nghttp3_conv.c:89:7: error: call to undeclared function 'htons'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
  n = htons(n);
      ^
7 errors generated.

I added -Wno-implicit-function-declaration option to cflags

[nodejs-github-bot]

Review requested:

• @nodejs/gyp

[mscdex]

This should be unnecessary as I believe this was already fixed upstream in v0.12.0. Our copy of nghttp3 just needs to be upgraded.

[bnoordhuis]

That's not a proper fix, it possibly doesn't even produce a functional build. Can you try this patch?

diff --git a/deps/ngtcp2/ngtcp2.gyp b/deps/ngtcp2/ngtcp2.gyp
index a47a791610..fd6eaddb01 100644
--- a/deps/ngtcp2/ngtcp2.gyp
+++ b/deps/ngtcp2/ngtcp2.gyp
@@ -112,7 +112,7 @@
             },
           },
         }],
-        ['OS=="linux"', {
+        ['OS in "android linux"', {
           'defines': [
             'HAVE_ARPA_INET_H',
             'HAVE_NETINET_IN_H',
@@ -162,7 +162,7 @@
             },
           },
         }],
-        ['OS=="linux"', {
+        ['OS in "android linux"', {
           'defines': [
             'HAVE_ARPA_INET_H',
             'HAVE_NETINET_IN_H',

[MatteoBax]

That's not a proper fix, it possibly doesn't even produce a functional build. Can you try this patch?

diff --git a/deps/ngtcp2/ngtcp2.gyp b/deps/ngtcp2/ngtcp2.gyp
index a47a791610..fd6eaddb01 100644
--- a/deps/ngtcp2/ngtcp2.gyp
+++ b/deps/ngtcp2/ngtcp2.gyp
@@ -112,7 +112,7 @@
             },
           },
         }],
-        ['OS=="linux"', {
+        ['OS in "android linux"', {
           'defines': [
             'HAVE_ARPA_INET_H',
             'HAVE_NETINET_IN_H',
@@ -162,7 +162,7 @@
             },
           },
         }],
-        ['OS=="linux"', {
+        ['OS in "android linux"', {
           'defines': [
             'HAVE_ARPA_INET_H',
             'HAVE_NETINET_IN_H',

With this patch those problems no longer occur, but now the problems are:

../deps/v8/src/base/debug/stack_trace_posix.cc:156:9: error: use of undeclared identifier 'backtrace_symbols'
        backtrace_symbols(trace, static_cast<int>(size)));
        ^
  /home/matteo/Android/Sdk/ndk/26.0.10792818/toolchains/llvm/prebuilt/linux-x86_64/bin/aarch64-linux-android24-clang++ -o /home/matteo/node/out/Release/obj.target/v8_libbase/deps/v8/src/base/division-by-constant.o ../deps/v8/src/base/division-by-constant.cc '-D_GLIBCXX_USE_CXX11_ABI=1' '-DNODE_OPENSSL_CONF_NAME=nodejs_conf' '-DNODE_OPENSSL_HAS_QUIC' '-DICU_NO_USER_DATA_OVERRIDE' '-DV8_GYP_BUILD' '-DV8_TYPED_ARRAY_MAX_SIZE_IN_HEAP=64' '-D__STDC_FORMAT_MACROS' '-DOPENSSL_NO_PINSHARED' '-DOPENSSL_THREADS' '-DOPENSSL_NO_ASM' '-DV8_TARGET_ARCH_ARM64' '-DV8_HAVE_TARGET_OS' '-DV8_TARGET_OS_ANDROID' '-DV8_EMBEDDER_STRING="-node.15"' '-DENABLE_DISASSEMBLER' '-DV8_PROMISE_INTERNAL_FIELD_COUNT=1' '-DOBJECT_PRINT' '-DV8_INTL_SUPPORT' '-DV8_ATOMIC_OBJECT_FIELD_WRITES' '-DV8_ENABLE_LAZY_SOURCE_POSITIONS' '-DV8_USE_SIPHASH' '-DV8_SHARED_RO_HEAP' '-DV8_WIN64_UNWINDING_INFO' '-DV8_ENABLE_REGEXP_INTERPRETER_THREADED_DISPATCH' '-DV8_USE_ZLIB' '-DV8_ENABLE_TURBOFAN' '-DV8_ENABLE_WEBASSEMBLY' '-DV8_ENABLE_JAVASCRIPT_PROMISE_HOOKS' '-DV8_ALLOCATION_FOLDING' '-DV8_ALLOCATION_SITE_TRACKING' '-DV8_SCRIPTORMODULE_LEGACY_LIFETIME' '-DV8_ADVANCED_BIGINT_ALGORITHMS' '-DBUILDING_V8_BASE_SHARED' '-D_GLIBCXX_USE_C99_MATH' -I../deps/v8 -I../deps/v8/include  -msign-return-address=all -Wno-unused-parameter -Wno-return-type -fno-omit-frame-pointer -fPIC -I/home/matteo/Android/Sdk/ndk/26.0.10792818/sources/android/cpufeatures -fdata-sections -ffunction-sections -O2 -fno-rtti -fno-exceptions -std=gnu++17 -MMD -MF /home/matteo/node/out/Release/.deps//home/matteo/node/out/Release/obj.target/v8_libbase/deps/v8/src/base/division-by-constant.o.d.raw   -c
../deps/v8/src/base/debug/stack_trace_posix.cc:371:32: error: use of undeclared identifier 'backtrace'; did you mean 'StackTrace'?
  count_ = static_cast<size_t>(backtrace(trace_, arraysize(trace_)));
                               ^
../deps/v8/src/base/debug/stack_trace.h:41:22: note: 'StackTrace' declared here
class V8_BASE_EXPORT StackTrace {
                     ^
2 errors generated.
make[1]: *** [tools/v8_gypfiles/v8_libbase.host.mk:186: /home/matteo/node/out/Release/obj.host/v8_libbase/deps/v8/src/base/debug/stack_trace_posix.o] Errore 1
make[1]: *** Attesa per i processi non terminati....

[bnoordhuis]

Covered in #46952.

[MatteoBax]

Covered in #46952.

Ok. I revert the changes and applied the patch. Instead of using 'OS in "android linux"' I used 'OS=="linux" or OS=="android"' but I think it's the same thing.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/54435/

[MatteoBax]

What shall I do to fix failed checks?

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/54438/

[bnoordhuis]

They're flaky tests. Not much you can do.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/54444/

[nodejs-github-bot]

Landed in 61411bb