address CR comments

nodejs · Jul 23, 2016 · e1c6b64 · e1c6b64
1 parent e1aeabe
commit e1c6b64
Show file tree

Hide file tree

Showing 4 changed files with 76 additions and 76 deletions.
diff --git a/deps/chakrashim/chakracore.gyp b/deps/chakrashim/chakracore.gyp
@@ -115,8 +115,6 @@
         'library_dirs': [ '<(PRODUCT_DIR)' ],
         'conditions': [
           ['OS=="win"', {
-            'libraries': [
-            ],
           }, {
             'libraries': [
               '-Wl,--no-undefined',

diff --git a/deps/chakrashim/core/lib/Jsrt/ChakraCommon.h b/deps/chakrashim/core/lib/Jsrt/ChakraCommon.h
@@ -2670,21 +2670,23 @@ typedef UINT32 DWORD;
     CHAKRA_API
         JsWriteString(
             JsValueRef value,
+            int start,
+            int length,
             char* buffer,
-            size_t bufferSize,
-            _Out_opt_ size_t* length);
+            _Out_opt_ size_t* written);
 
     CHAKRA_API
-        JsWriteStringUtf8(
+        JsWriteStringUtf16(
             JsValueRef value,
-            uint8_t* buffer,
-            size_t bufferSize,
-            _Out_opt_ size_t* length);
+            int start,
+            int length,
+            uint16_t* buffer,
+            _Out_opt_ size_t* written);
 
     CHAKRA_API
-        JsWriteStringUtf16(
+        JsWriteStringUtf8(
             JsValueRef value,
-            uint16_t* buffer,
+            uint8_t* buffer,
             size_t bufferSize,
             _Out_opt_ size_t* length);
 

diff --git a/deps/chakrashim/core/lib/Jsrt/Jsrt.cpp b/deps/chakrashim/core/lib/Jsrt/Jsrt.cpp
@@ -3492,12 +3492,19 @@ CHAKRA_API JsCreateStringUtf16(
 }
 
 
-CHAKRA_API JsWriteString(
+template <class CopyFunc>
+JsErrorCode WriteStringCopy(
     JsValueRef value,
-    char* buffer,
-    size_t bufferSize,
-    _Out_opt_ size_t* length)
+    int start,
+    int length,
+    _Out_opt_ size_t* written,
+    const CopyFunc& copyFunc)
 {
+    if (written)
+    {
+        *written = 0;  // init to 0 for default
+    }
+
     const char16* str = nullptr;
     size_t strLength = 0;
     JsErrorCode errorCode = JsStringToPointer(value, &str, &strLength);
@@ -3506,26 +3513,63 @@ CHAKRA_API JsWriteString(
         return errorCode;
     }
 
-    if (!buffer)
+    if (start < 0 || (size_t)start > strLength)
     {
-        if (length)
-        {
-            *length = strLength;
-        }
+        return JsNoError;  // start out of range, no chars written
     }
-    else
+
+    size_t count = min(static_cast<size_t>(length), strLength - start);
+    if (count == 0)
     {
-        size_t count = min(bufferSize, strLength);
-        CastCopy(str, buffer, count);
-        if (length)
-        {
-            *length = count;
-        }
+        return JsNoError;  // no chars written
+    }
+
+    errorCode = copyFunc(str + start, count);
+    if (errorCode != JsNoError)
+    {
+        return errorCode;
+    }
+
+    if (written)
+    {
+        *written = count;
     }
 
     return JsNoError;
 }
 
+CHAKRA_API JsWriteString(
+    JsValueRef value,
+    int start,
+    int length,
+    char* buffer,
+    _Out_opt_ size_t* written)
+{
+    return WriteStringCopy(value, start, length, written,
+        [buffer](const char16* src, size_t count)
+        {
+            PARAM_NOT_NULL(buffer);
+            CastCopy(src, buffer, count);
+            return JsNoError;
+        });
+}
+
+CHAKRA_API JsWriteStringUtf16(
+    JsValueRef value,
+    int start,
+    int length,
+    uint16_t* buffer,
+    _Out_opt_ size_t* written)
+{
+    return WriteStringCopy(value, start, length, written,
+        [buffer](const char16* src, size_t count)
+        {
+            PARAM_NOT_NULL(buffer);
+            memmove(buffer, src, sizeof(char16) * count);
+            return JsNoError;
+        });
+}
+
 CHAKRA_API JsWriteStringUtf8(
     JsValueRef value,
     uint8_t* buffer,
@@ -3570,40 +3614,6 @@ CHAKRA_API JsWriteStringUtf8(
     return JsNoError;
 }
 
-CHAKRA_API JsWriteStringUtf16(
-    JsValueRef value,
-    uint16_t* buffer,
-    size_t bufferSize,
-    _Out_opt_ size_t* length)
-{
-    const char16* str = nullptr;
-    size_t strLength = 0;
-    JsErrorCode errorCode = JsStringToPointer(value, &str, &strLength);
-    if (errorCode != JsNoError)
-    {
-        return errorCode;
-    }
-
-    if (!buffer)
-    {
-        if (length)
-        {
-            *length = strLength;
-        }
-    }
-    else
-    {
-        size_t count = min(bufferSize, strLength);
-        memmove(buffer, str, sizeof(char16) * count);
-        if (length)
-        {
-            *length = count;
-        }
-    }
-
-    return JsNoError;
-}
-
 
 /////////////////////
 

diff --git a/deps/chakrashim/src/v8string.cc b/deps/chakrashim/src/v8string.cc
@@ -83,15 +83,10 @@ int String::Utf8Length() const {
 }
 
 int String::Write(uint16_t *buffer, int start, int length, int options) const {
-  if (length < 0) {
-    // in case length was not provided we want to copy the whole string
-    length = String::kMaxLength;
-  }
-
   size_t count = 0;
-  if (JsWriteStringUtf16((JsValueRef)this,
-                         buffer + start, length, &count) == JsNoError) {
-    if (count < (unsigned)length && !(options & String::NO_NULL_TERMINATION)) {
+  if (JsWriteStringUtf16((JsValueRef)this, start, length,
+                         buffer, &count) == JsNoError) {
+    if (!(options & String::NO_NULL_TERMINATION)) {
       buffer[count] = 0;
     }
   }
@@ -100,15 +95,10 @@ int String::Write(uint16_t *buffer, int start, int length, int options) const {
 
 int String::WriteOneByte(
     uint8_t* buffer, int start, int length, int options) const {
-  if (length < 0) {
-    // in case length was not provided we want to copy the whole string
-    length = String::kMaxLength;
-  }
-
   size_t count = 0;
-  if (JsWriteString((JsValueRef)this,
-    (char*)buffer + start, length, &count) == JsNoError) {
-    if (count < (unsigned)length && !(options & String::NO_NULL_TERMINATION)) {
+  if (JsWriteString((JsValueRef)this, start, length,
+                    (char*)buffer, &count) == JsNoError) {
+    if (!(options & String::NO_NULL_TERMINATION)) {
       buffer[count] = 0;
     }
   }