Skip to content
This repository has been archived by the owner on Aug 25, 2022. It is now read-only.

Got undeclared identifier 'htonll' when building on Mac OS X 10.9 #4

Closed
lianghe-houzz opened this issue Sep 1, 2015 · 9 comments
Closed
Labels

Comments

@lianghe-houzz
Copy link

/tmp/hhvm20150831-52889-miidu8/hhvm-3.9.1/hphp/runtime/ext/fb/ext_fb.cpp:316:21: error: use of undeclared identifier 'htonll'; did you mean 'atoll'?
3241     uint64_t nval = htonll(kInt54Prefix | val);
3242                     ^~~~~~
3243                     atoll

Looks like need a check similar as in hphp/runtime/base/thrift-buffer.h:39-60 ?

@jwatzman jwatzman added the bug label Sep 1, 2015
@jwatzman
Copy link
Contributor

jwatzman commented Sep 1, 2015

Interesting, will look into it later.

@jwatzman
Copy link
Contributor

jwatzman commented Sep 1, 2015

Looks like need a check similar as in hphp/runtime/base/thrift-buffer.h:39-60 ?

Yeah, it's unclear why this file has its own implementations (maybe there is a good reason, we'll see); we can probably move the one in thrift-buffer.h into portability.h or something.

@jwatzman
Copy link
Contributor

jwatzman commented Sep 1, 2015

But also curious why this is working on my machine. Maybe something with __BYTE_ORDER on 10.9 vs 10.10.

@wjywbs
Copy link

wjywbs commented Sep 2, 2015

htonll is defined in 10.10 at /usr/include/sys/_endian.h, but not in 10.9.

@jwatzman
Copy link
Contributor

jwatzman commented Sep 2, 2015

Attempted fix is up internally. I don't actually have a 10.9 machine to test with though...

@lianghe-houzz
Copy link
Author

I tried to patch ext_fb.cpp like what I said, and can be successfully built without other change. :)

@jwatzman
Copy link
Contributor

jwatzman commented Sep 2, 2015

Yeah that's what my internal diff does, more or less :) Running into some infra issues right now, so it might be a couple of days to get it out since I'm in the middle of some travel right now.

@jwatzman
Copy link
Contributor

jwatzman commented Sep 2, 2015

Here's my patch, since it could be a couple of days. If you could confirm this patch fixes the issue, that would actually be really useful since I don't have a 10.9 machine and am just kind of guessing based on what you and @wjywbs said.

commit 7a0ccb5c8162826ae0ed3cf9531f62568147d2db
Author: Josh Watzman <jwatzman@fb.com>
Date:   Wed Sep 2 11:41:58 2015 -0700

    [hhvm] Only one compatibility implementation of htonll

    Summary:
    We have two, but one is much less featureful than the other,
    and is also implemented as two 32-bit swaps (when some libc-provided
    implementations can just use a single asm instruction sometimes). Let's
    just use one.

    I *hope* this will also fix the build on OS X 10.9, where `htonll` isn't
    defined, by standardizing on the macro that actually works there. But I
    don't have a system to test on.

    Ref. https://github.com/hhvm/homebrew-hhvm/issues/4

diff --git a/hphp/runtime/base/thrift-buffer.h b/hphp/runtime/base/thrift-buffer.h
index cca13dd..f44a696 100644
--- a/hphp/runtime/base/thrift-buffer.h
+++ b/hphp/runtime/base/thrift-buffer.h
@@ -19,45 +19,7 @@

 #include "hphp/runtime/base/type-string.h"
 #include "hphp/runtime/base/variable-serializer.h"
-
-#include <arpa/inet.h>
-#if defined(__FreeBSD__)
-# include <sys/endian.h>
-#elif defined(__APPLE__)
-# include <machine/endian.h>
-# include <libkern/OSByteOrder.h>
-#elif defined(_MSC_VER)
-# include <stdlib.h>
-#else
-# include <byteswap.h>
-# include <map>
-# include <memory>
-# include <utility>
-# include <vector>
-#endif
-
-#if !defined(htonll) && !defined(ntohll)
-
-#if __BYTE_ORDER == __LITTLE_ENDIAN
-# if defined(__FreeBSD__)
-#  define htonll(x) bswap64(x)
-#  define ntohll(x) bswap64(x)
-# elif defined(__APPLE__)
-#  define htonll(x) OSSwapInt64(x)
-#  define ntohll(x) OSSwapInt64(x)
-# elif defined(_MSC_VER)
-#  define htonll(x) _byteswap_uint64(x)
-#  define ntohll(x) _byteswap_uint64(x)
-# else
-#  define htonll(x) bswap_64(x)
-#  define ntohll(x) bswap_64(x)
-# endif
-#else
-# define htonll(x) (x)
-# define ntohll(x) (x)
-#endif
-
-#endif
+#include "hphp/util/htonll.h"

 namespace HPHP {
 ///////////////////////////////////////////////////////////////////////////////
diff --git a/hphp/runtime/ext/fb/ext_fb.cpp b/hphp/runtime/ext/fb/ext_fb.cpp
index a30331b..36c0fda 100644
--- a/hphp/runtime/ext/fb/ext_fb.cpp
+++ b/hphp/runtime/ext/fb/ext_fb.cpp
@@ -29,6 +29,7 @@

 #include <folly/String.h>

+#include "hphp/util/htonll.h"
 #include "hphp/util/logger.h"
 #include "hphp/runtime/base/array-init.h"
 #include "hphp/runtime/base/builtin-functions.h"
@@ -76,24 +77,6 @@ const StaticString

 ///////////////////////////////////////////////////////////////////////////////

-#if defined __BYTE_ORDER && defined __BIG_ENDIAN
-/* Linux and other systems don't currently support a ntohx or htonx
-   set of functions for 64-bit values.  We've implemented our own here
-   which is based off of GNU Net's implementation with some slight
-   modifications (changed to macro's rather than functions). */
-#if __BYTE_ORDER == __BIG_ENDIAN
-#define ntohll(n) (n)
-#define htonll(n) (n)
-#else
-#define ntohll(n)                                                       \
-  ( (((uint64_t)ntohl(n)) << 32)                                        \
-    | ((uint64_t)ntohl((n) >> 32) & 0x00000000ffffffff) )
-#define htonll(n)                                                       \
-  ( (((uint64_t)htonl(n)) << 32)                                        \
-    | ((uint64_t)htonl((n) >> 32) & 0x00000000ffffffff) )
-#endif
-#endif
-
 /* enum of thrift types */
 enum TType {
   T_STOP    = 1,
diff --git a/hphp/util/htonll.h b/hphp/util/htonll.h
new file mode 100644
index 0000000..cb9e0cc
--- /dev/null
+++ b/hphp/util/htonll.h
@@ -0,0 +1,61 @@
+/*
+   +----------------------------------------------------------------------+
+   | HipHop for PHP                                                       |
+   +----------------------------------------------------------------------+
+   | Copyright (c) 2010-2015 Facebook, Inc. (http://www.facebook.com)     |
+   +----------------------------------------------------------------------+
+   | This source file is subject to version 3.01 of the PHP license,      |
+   | that is bundled with this package in the file LICENSE, and is        |
+   | available through the world-wide-web at the following url:           |
+   | http://www.php.net/license/3_01.txt                                  |
+   | If you did not receive a copy of the PHP license and are unable to   |
+   | obtain it through the world-wide-web, please send a note to          |
+   | license@php.net so we can mail you a copy immediately.               |
+   +----------------------------------------------------------------------+
+*/
+
+#ifndef incl_HPHP_HTONLL_H_
+#define incl_HPHP_HTONLL_H_
+
+/*
+ * Tries to find a suitable implementation of htonll/ntohll if it doesn't
+ * already exist. This could go into portability.h, but seemed specific enough
+ * to be worth pulling out.
+ */
+
+#include <arpa/inet.h>
+#if defined(__FreeBSD__)
+# include <sys/endian.h>
+#elif defined(__APPLE__)
+# include <machine/endian.h>
+# include <libkern/OSByteOrder.h>
+#elif defined(_MSC_VER)
+# include <stdlib.h>
+#else
+# include <byteswap.h>
+#endif
+
+#if !defined(htonll) && !defined(ntohll)
+
+#if __BYTE_ORDER == __LITTLE_ENDIAN
+# if defined(__FreeBSD__)
+#  define htonll(x) bswap64(x)
+#  define ntohll(x) bswap64(x)
+# elif defined(__APPLE__)
+#  define htonll(x) OSSwapInt64(x)
+#  define ntohll(x) OSSwapInt64(x)
+# elif defined(_MSC_VER)
+#  define htonll(x) _byteswap_uint64(x)
+#  define ntohll(x) _byteswap_uint64(x)
+# else
+#  define htonll(x) bswap_64(x)
+#  define ntohll(x) bswap_64(x)
+# endif
+#else
+# define htonll(x) (x)
+# define ntohll(x) (x)
+#endif
+
+#endif
+
+#endif

jwatzman added a commit to facebook/hhvm that referenced this issue Sep 4, 2015
Summary: We have two, but one is much less featureful than the other,
and is also implemented as two 32-bit swaps (when some libc-provided
implementations can just use a single asm instruction sometimes). Let's
just use one.

I *hope* this will also fix the build on OS X 10.9, where `htonll` isn't
defined, by standardizing on the macro that actually works there. But I
don't have a system to test on.

Ref. hhvm/homebrew-hhvm#4

Reviewed By: @paulbiss

Differential Revision: D2407062
@jwatzman
Copy link
Contributor

jwatzman commented Sep 8, 2015

This should be fixed if you build with --HEAD. Closing out, LMK if it's still not working.

@jwatzman jwatzman closed this as completed Sep 8, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants