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

Tweak nghttp3_ntohl64/nghttp3_htonl64 definitions #135

Merged
merged 1 commit into from
May 29, 2023
Merged

Conversation

tatsuhiro-t
Copy link
Member

  • Only define nghttp3_bswap64 for little endian platform.
  • Use _byteswap_uint64 for win32.
  • Use OSSwapInt64 for apple platform.

- Only define nghttp3_bswap64 for little endian platform.
- Use _byteswap_uint64 for win32.
- Use OSSwapInt64 for apple platform.
@tatsuhiro-t tatsuhiro-t added this to the v0.12.0 milestone May 29, 2023
@tatsuhiro-t tatsuhiro-t merged commit 5a473ad into main May 29, 2023
26 checks passed
@tatsuhiro-t tatsuhiro-t deleted the tweak-ntohl64 branch May 29, 2023 09:05
# define nghttp3_bswap64 OSSwapInt64
# else /* !HAVE_BSWAP_64 && !WIN32 && !__APPLE__ */
# define nghttp3_bswap64(N) \
((uint64_t)(nghttp3_ntohl((uint32_t)(N))) << 32 | \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be a bug for Android. We got the following compile error after rebasing to this commit.

ld.lld: error: undefined symbol: nghttp3_ntohl
>>> referenced by nghttp3_conv.c:58 (third-party/nghttp3/lib/nghttp3_conv.c:58)
>>>               ../nghttp3#compile-pic-nghttp3_conv.c.o2a37e81d,platform010-clang/lib/nghttp3_conv.c.o:(nghttp3_get_varint) 

There is no function or macro for nghttp3_ntohl. The following fixes the compile error.

diff --git a/third-party/nghttp3/lib/nghttp3_conv.h b/third-party/nghttp3/lib/nghttp3_conv.h
--- a/third-party/nghttp3/lib/nghttp3_conv.h
+++ b/third-party/nghttp3/lib/nghttp3_conv.h
@@ -74,8 +74,8 @@
 #      define nghttp3_bswap64 OSSwapInt64
 #    else /* !HAVE_BSWAP_64 && !WIN32 && !__APPLE__ */
 #      define nghttp3_bswap64(N)                                               \
-        ((uint64_t)(nghttp3_ntohl((uint32_t)(N))) << 32 |                      \
-         nghttp3_ntohl((uint32_t)((N) >> 32)))
+        ((uint64_t)(ntohl((uint32_t)(N))) << 32 |                      \
+         ntohl((uint32_t)((N) >> 32)))
 #    endif /* !HAVE_BSWAP_64 && !WIN32 && !__APPLE__ */
 #    define nghttp3_ntohl64(N) nghttp3_bswap64(N)
 #    define nghttp3_htonl64(N) nghttp3_bswap64(N)

lhuang04 added a commit to lhuang04/nghttp3 that referenced this pull request May 31, 2023
Summary:
Fix compile error for Android/Linux - undefined symbol: nghttp3_ntohl

Summary:
Found the following compile error when we build the latest nghttp3 for
Android/Linux.

```
ld.lld: error: undefined symbol: nghttp3_ntohl >>> referenced by nghttp3_conv.c:58 (third-party/nghttp3/lib/nghttp3_conv.c:58) >>> ../nghttp3#compile-pic-nghttp3_conv.c.o2a37e81d,platform010-clang/lib/nghttp3_conv.c.o:(nghttp3_get_varint)
```

nghttp3_ntohl is not a function or macro. We should use ntohl instead
which is also part of previos logic in ngtcp2#135.

Test Plan:
Compile and check no error.

Reviewers:

Subscribers:

Tasks:

Tags:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants