Skip to content

Commit

Permalink
deps,src: align ssize_t ABI between Node & nghttp2
Browse files Browse the repository at this point in the history
Previously, we performed casts that are considered undefined behavior.
Instead, just define `ssize_t` for nghttp2 the same way we define it
for the rest of Node.

Also, remove a TODO comment that would probably also be *technically*
correct but shouldn’t matter as long as nobody is complaining.

Backport-PR-URL: #20456
PR-URL: #18565
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
addaleax authored and MylesBorins committed May 15, 2018
1 parent 801a499 commit 5394bc5
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 11 deletions.
14 changes: 12 additions & 2 deletions deps/nghttp2/lib/includes/config.h
Original file line number Diff line number Diff line change
@@ -1,8 +1,18 @@
/* Hint to the compiler that a function never returns */
#define NGHTTP2_NORETURN

/* Define to `int' if <sys/types.h> does not define. */
#define ssize_t int
/* Edited to match src/node.h. */
#include <stdint.h>

#ifdef _WIN32
#if !defined(_SSIZE_T_) && !defined(_SSIZE_T_DEFINED)
typedef intptr_t ssize_t;
# define _SSIZE_T_
# define _SSIZE_T_DEFINED
#endif
#else // !_WIN32
# include <sys/types.h> // size_t, ssize_t
#endif // _WIN32

/* Define to 1 if you have the `std::map::emplace`. */
#define HAVE_STD_MAP_EMPLACE 1
Expand Down
1 change: 0 additions & 1 deletion src/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,6 @@ NODE_EXTERN v8::Local<v8::Value> MakeCallback(
#endif

#ifdef _WIN32
// TODO(tjfontaine) consider changing the usage of ssize_t to ptrdiff_t
#if !defined(_SSIZE_T_) && !defined(_SSIZE_T_DEFINED)
typedef intptr_t ssize_t;
# define _SSIZE_T_
Expand Down
10 changes: 2 additions & 8 deletions src/node_http2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -775,13 +775,7 @@ inline ssize_t Http2Session::Write(const uv_buf_t* bufs, size_t nbufs) {
bufs[n].len);
CHECK_NE(ret, NGHTTP2_ERR_NOMEM);

// If there is an error calling any of the callbacks, ret will be a
// negative number identifying the error code. This can happen, for
// instance, if the session is destroyed during any of the JS callbacks
// Note: if ssize_t is not defined (e.g. on Win32), nghttp2 will typedef
// ssize_t to int. Cast here so that the < 0 check actually works on
// Windows.
if (static_cast<int>(ret) < 0)
if (ret < 0)
return ret;

total += ret;
Expand Down Expand Up @@ -1693,7 +1687,7 @@ void Http2Session::OnStreamReadImpl(ssize_t nread,
// ssize_t to int. Cast here so that the < 0 check actually works on
// Windows.
if (static_cast<int>(ret) < 0) {
DEBUG_HTTP2SESSION2(session, "fatal error receiving data: %d", ret);
DEBUG_HTTP2SESSION2(this, "fatal error receiving data: %d", ret);

Local<Value> argv[1] = {
Integer::New(isolate, ret),
Expand Down

0 comments on commit 5394bc5

Please sign in to comment.