From 8beb4bb58ab93af8c95af6844230d62c85ccab78 Mon Sep 17 00:00:00 2001 From: "REDMOND\\acoates" Date: Tue, 5 Feb 2019 06:09:16 -0800 Subject: [PATCH] Various minor changes to allow compliation with msvc (#22182) Summary: This change is aiming to reduce some of the forking changes we have internally in order to use CxxReact for some additional out of tree platforms. Some of the fixes allow more of the code to compile when using Microsoft Visual Studio Compiler. In particular the change around the default value of RN_EXPORT and some changes around how to enable the packing attribute. Another change moves more of the code for JSBigFileString into the cpp file, so that people can share the header but replace the implementation as appropriate for other platforms. And finally the removal of an unused header include. This is unlikely to be the extent of the changes required for MSVC, but at least gets one of our complication blocks to work against an unforked RN. Pull Request resolved: https://github.com/facebook/react-native/pull/22182 Differential Revision: D12967758 Pulled By: cpojer fbshipit-source-id: a2cc018aedaa9916cd644bfbd9e3a55330cd4c52 --- ReactCommon/cxxreact/JSBigString.cpp | 54 +++++++++++++++++++ ReactCommon/cxxreact/JSBigString.h | 63 ++++------------------ ReactCommon/cxxreact/JSBundleType.h | 5 +- ReactCommon/cxxreact/RAMBundleRegistry.cpp | 2 - ReactCommon/cxxreact/tests/BUCK | 2 + 5 files changed, 71 insertions(+), 55 deletions(-) diff --git a/ReactCommon/cxxreact/JSBigString.cpp b/ReactCommon/cxxreact/JSBigString.cpp index e4b806f434..e26a81008b 100644 --- a/ReactCommon/cxxreact/JSBigString.cpp +++ b/ReactCommon/cxxreact/JSBigString.cpp @@ -9,11 +9,65 @@ #include #include +#include #include namespace facebook { namespace react { +JSBigFileString::JSBigFileString(int fd, size_t size, off_t offset /*= 0*/) + : m_fd { -1 } + , m_data { nullptr } { + folly::checkUnixError(m_fd = dup(fd), + "Could not duplicate file descriptor"); + + // Offsets given to mmap must be page aligend. We abstract away that + // restriction by sending a page aligned offset to mmap, and keeping track + // of the offset within the page that we must alter the mmap pointer by to + // get the final desired offset. + if (offset != 0) { + const static auto ps = getpagesize(); + auto d = lldiv(offset, ps); + + m_mapOff = d.quot; + m_pageOff = d.rem; + m_size = size + m_pageOff; + } else { + m_mapOff = 0; + m_pageOff = 0; + m_size = size; + } +} + +JSBigFileString::~JSBigFileString() { + if (m_data) { + munmap((void *)m_data, m_size); + } + close(m_fd); +} + + +const char *JSBigFileString::c_str() const { + if (!m_data) { + m_data = + (const char *) mmap(0, m_size, PROT_READ, MAP_SHARED, m_fd, m_mapOff); + CHECK(m_data != MAP_FAILED) + << " fd: " << m_fd + << " size: " << m_size + << " offset: " << m_mapOff + << " error: " << std::strerror(errno); + } + return m_data + m_pageOff; +} + +size_t JSBigFileString::size() const { + return m_size - m_pageOff; +} + +int JSBigFileString::fd() const { + return m_fd; +} + std::unique_ptr JSBigFileString::fromPath(const std::string& sourceURL) { int fd = ::open(sourceURL.c_str(), O_RDONLY); folly::checkUnixError(fd, "Could not open file", sourceURL); diff --git a/ReactCommon/cxxreact/JSBigString.h b/ReactCommon/cxxreact/JSBigString.h index 55bc7e8ed1..97a350d3ac 100644 --- a/ReactCommon/cxxreact/JSBigString.h +++ b/ReactCommon/cxxreact/JSBigString.h @@ -13,7 +13,11 @@ #include #ifndef RN_EXPORT -#define RN_EXPORT __attribute__((visibility("default"))) +# ifdef _MSC_VER +# define RN_EXPORT +# else +# define RN_EXPORT __attribute__((visibility("default"))) +# endif #endif namespace facebook { @@ -74,7 +78,7 @@ private: // buffer, and provides an accessor for writing to it. This can be // used to construct a JSBigString in place, such as by reading from a // file. -class JSBigBufferString : public JSBigString { +class RN_EXPORT JSBigBufferString : public JSBigString { public: JSBigBufferString(size_t size) : m_data(new char[size + 1]) @@ -113,62 +117,17 @@ private: class RN_EXPORT JSBigFileString : public JSBigString { public: - JSBigFileString(int fd, size_t size, off_t offset = 0) - : m_fd {-1} - , m_data {nullptr} - { - folly::checkUnixError(m_fd = dup(fd), - "Could not duplicate file descriptor"); - - // Offsets given to mmap must be page aligend. We abstract away that - // restriction by sending a page aligned offset to mmap, and keeping track - // of the offset within the page that we must alter the mmap pointer by to - // get the final desired offset. - if (offset != 0) { - const static auto ps = getpagesize(); - auto d = lldiv(offset, ps); - - m_mapOff = d.quot; - m_pageOff = d.rem; - m_size = size + m_pageOff; - } else { - m_mapOff = 0; - m_pageOff = 0; - m_size = size; - } - } - - ~JSBigFileString() { - if (m_data) { - munmap((void *)m_data, m_size); - } - close(m_fd); - } + JSBigFileString(int fd, size_t size, off_t offset = 0); + ~JSBigFileString(); bool isAscii() const override { return true; } - const char *c_str() const override { - if (!m_data) { - m_data = - (const char *)mmap(0, m_size, PROT_READ, MAP_PRIVATE, m_fd, m_mapOff); - CHECK(m_data != MAP_FAILED) - << " fd: " << m_fd - << " size: " << m_size - << " offset: " << m_mapOff - << " error: " << std::strerror(errno); - } - return m_data + m_pageOff; - } + const char *c_str() const override; - size_t size() const override { - return m_size - m_pageOff; - } - - int fd() const { - return m_fd; - } + size_t size() const override; + int fd() const; static std::unique_ptr fromPath(const std::string& sourceURL); diff --git a/ReactCommon/cxxreact/JSBundleType.h b/ReactCommon/cxxreact/JSBundleType.h index 530456a98a..0cd18948a6 100644 --- a/ReactCommon/cxxreact/JSBundleType.h +++ b/ReactCommon/cxxreact/JSBundleType.h @@ -7,6 +7,7 @@ #include #include +#include #ifndef RN_EXPORT #define RN_EXPORT __attribute__((visibility("default"))) @@ -34,7 +35,8 @@ enum struct ScriptTag { * 4 bytes, for BC bundles this is 12 bytes. This structure holds the first 12 * bytes from a bundle in a way that gives access to that information. */ -struct __attribute__((packed)) BundleHeader { +FOLLY_PACK_PUSH +struct FOLLY_PACK_ATTR BundleHeader { BundleHeader() { std::memset(this, 0, sizeof(BundleHeader)); } @@ -43,6 +45,7 @@ struct __attribute__((packed)) BundleHeader { uint32_t reserved_; uint32_t version; }; +FOLLY_PACK_POP /** * parseTypeFromHeader diff --git a/ReactCommon/cxxreact/RAMBundleRegistry.cpp b/ReactCommon/cxxreact/RAMBundleRegistry.cpp index 05c5ad2155..f53370d3c4 100644 --- a/ReactCommon/cxxreact/RAMBundleRegistry.cpp +++ b/ReactCommon/cxxreact/RAMBundleRegistry.cpp @@ -8,8 +8,6 @@ #include #include -#include - namespace facebook { namespace react { diff --git a/ReactCommon/cxxreact/tests/BUCK b/ReactCommon/cxxreact/tests/BUCK index 8e2282f24a..25c4d3506c 100644 --- a/ReactCommon/cxxreact/tests/BUCK +++ b/ReactCommon/cxxreact/tests/BUCK @@ -32,6 +32,7 @@ jni_instrumentation_test_lib( "xplat//third-party/linker_lib:android", "xplat//third-party/linker_lib:atomic", react_native_xplat_target("cxxreact:bridge"), + react_native_xplat_target("cxxreact:jsbigstring"), ], ) @@ -50,5 +51,6 @@ fb_xplat_cxx_test( "xplat//folly:molly", "xplat//third-party/gmock:gtest", react_native_xplat_target("cxxreact:bridge"), + react_native_xplat_target("cxxreact:jsbigstring"), ], )