Skip to content

Commit

Permalink
[MERGE #3433 @kunalspathak] Jsrt: Modify signature of JsCopyString
Browse files Browse the repository at this point in the history
Merge pull request #3433 from pr/kunalspathak/1.7

Modified signature of `JsCopyString` to also return actual count of UTF8 bytes present in jsString.
With this information, host can simply allocate a buffer assuming all characters are ascii and
based on `writtenLength` and `actualLength` values returned by the API, it can decide if the assumption was correct
i.e. `writtenLength == actualLength` or it should take slow path to call `JsCopyString` again by passing bigger buffer
equal to size of`actualLength`.

Today, if host wants to copy UTF8 representation of javascript string into a buffer, here is how it is done:

```c++
   size_t length = 0;
   JsCopyString(jsString, nullptr, 0, &length);

   char* buffer = malloc(length);

   size_t written = 0;
   JsCopyString(jsString, buffer, length, &written);
   assert(written == length);
```

can be changed to

```c++
   size_t actualLength = 0;
   size_t writtenLength = 0;
   size_t strLength = 0;
   JsStringToPointer(strRef, nullptr, &strLength);
   char* buffer = malloc(strLength + 1);
   JsCopyString(jsString, buffer, strLength, &writtenLength, &actualLength);

   // slow path if jsString contains non-ascii characters
   if(writtenLength != actualLength) {
      free(buffer);
      buffer = malloc(actualLength + 1);

      JsCopyString(jsString, buffer, actualLength, &writtenLength, &actualLength);
   }
```
  • Loading branch information
kunalspathak committed Jul 27, 2017
2 parents 1ffb64c + 356656c commit 3c7ecf0
Show file tree
Hide file tree
Showing 11 changed files with 71 additions and 32 deletions.
1 change: 1 addition & 0 deletions bin/ch/ChakraRtInterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ bool ChakraRTInterface::LoadChakraDll(ArgInfo* argInfo, HINSTANCE *outLibrary)
m_jsApiHooks.pfJsrtParse = (JsAPIHooks::JsrtParse)GetChakraCoreSymbol(library, "JsParse");
m_jsApiHooks.pfJsrtSerialize = (JsAPIHooks::JsrtSerialize)GetChakraCoreSymbol(library, "JsSerialize");
m_jsApiHooks.pfJsrtRunSerialized = (JsAPIHooks::JsrtRunSerialized)GetChakraCoreSymbol(library, "JsRunSerialized");
m_jsApiHooks.pfJsrtGetStringLength = (JsAPIHooks::JsrtGetStringLength)GetChakraCoreSymbol(library, "JsGetStringLength");
m_jsApiHooks.pfJsrtCreateString = (JsAPIHooks::JsrtCreateString)GetChakraCoreSymbol(library, "JsCreateString");
m_jsApiHooks.pfJsrtCreateStringUtf16 = (JsAPIHooks::JsrtCreateStringUtf16)GetChakraCoreSymbol(library, "JsCreateStringUtf16");
m_jsApiHooks.pfJsrtCopyString = (JsAPIHooks::JsrtCopyString)GetChakraCoreSymbol(library, "JsCopyString");
Expand Down
7 changes: 5 additions & 2 deletions bin/ch/ChakraRtInterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ struct JsAPIHooks
typedef JsErrorCode(WINAPI *JsrtParse)(JsValueRef script, JsSourceContext sourceContext, JsValueRef sourceUrl, JsParseScriptAttributes parseAttributes, JsValueRef *result);
typedef JsErrorCode(WINAPI *JsrtSerialize)(JsValueRef script, JsValueRef *buffer, JsParseScriptAttributes parseAttributes);
typedef JsErrorCode(WINAPI *JsrtRunSerialized)(JsValueRef buffer, JsSerializedLoadScriptCallback scriptLoadCallback, JsSourceContext sourceContext, JsValueRef sourceUrl, JsValueRef * result);
typedef JsErrorCode(WINAPI *JsrtCopyString)(JsValueRef value, char* buffer, size_t bufferSize, size_t* written);
typedef JsErrorCode(WINAPI *JsrtGetStringLength)(JsValueRef value, int *stringLength);
typedef JsErrorCode(WINAPI *JsrtCopyString)(JsValueRef value, char* buffer, size_t bufferSize, size_t* writtenLength, size_t* actualLength);
typedef JsErrorCode(WINAPI *JsrtCreateString)(const char *content, size_t length, JsValueRef *value);
typedef JsErrorCode(WINAPI *JsrtCreateStringUtf16)(const uint16_t *content, size_t length, JsValueRef *value);

Expand Down Expand Up @@ -172,6 +173,7 @@ struct JsAPIHooks
JsrtParse pfJsrtParse;
JsrtSerialize pfJsrtSerialize;
JsrtRunSerialized pfJsrtRunSerialized;
JsrtGetStringLength pfJsrtGetStringLength;
JsrtCreateString pfJsrtCreateString;
JsrtCreateStringUtf16 pfJsrtCreateStringUtf16;
JsrtCopyString pfJsrtCopyString;
Expand Down Expand Up @@ -396,7 +398,8 @@ class ChakraRTInterface
static JsErrorCode WINAPI JsParse(JsValueRef script, JsSourceContext sourceContext, JsValueRef sourceUrl, JsParseScriptAttributes parseAttributes, JsValueRef *result) { return HOOK_JS_API(Parse(script, sourceContext, sourceUrl, parseAttributes, result)); }
static JsErrorCode WINAPI JsSerialize(JsValueRef script, JsValueRef *buffer, JsParseScriptAttributes parseAttributes) { return HOOK_JS_API(Serialize(script, buffer, parseAttributes)); }
static JsErrorCode WINAPI JsRunSerialized(JsValueRef buffer, JsSerializedLoadScriptCallback scriptLoadCallback, JsSourceContext sourceContext, JsValueRef sourceUrl, JsValueRef * result) { return HOOK_JS_API(RunSerialized(buffer, scriptLoadCallback, sourceContext, sourceUrl, result)); }
static JsErrorCode WINAPI JsCopyString(JsValueRef value, char* buffer, size_t bufferSize, size_t* written) { return HOOK_JS_API(CopyString(value, buffer, bufferSize, written)); }
static JsErrorCode WINAPI JsGetStringLength(JsValueRef value, int *stringLength) { return HOOK_JS_API(GetStringLength(value, stringLength)); }
static JsErrorCode WINAPI JsCopyString(JsValueRef value, char* buffer, size_t bufferSize, size_t* writtenLength, size_t* actualLength) { return HOOK_JS_API(CopyString(value, buffer, bufferSize, writtenLength, actualLength)); }
static JsErrorCode WINAPI JsCreateString(const char *content, size_t length, JsValueRef *value) { return HOOK_JS_API(CreateString(content, length, value)); }
static JsErrorCode WINAPI JsCreateStringUtf16(const uint16_t *content, size_t length, JsValueRef *value) { return HOOK_JS_API(CreateStringUtf16(content, length, value)); }
static JsErrorCode WINAPI JsCreatePropertyId(const char *name, size_t length, JsPropertyIdRef *propertyId) { return HOOK_JS_API(CreatePropertyId(name, length, propertyId)); }
Expand Down
33 changes: 27 additions & 6 deletions bin/ch/stdafx.h
Original file line number Diff line number Diff line change
Expand Up @@ -206,18 +206,39 @@ class AutoString
{
strValue = value;
}
int strLen = 0;
size_t writtenLen = 0;
size_t actualLen = 0;
if (errorCode == JsNoError)
{
size_t len = 0;
errorCode = ChakraRTInterface::JsCopyString(strValue, nullptr, 0, &len);
errorCode = ChakraRTInterface::JsGetStringLength(strValue, &strLen);
if (errorCode == JsNoError)
{
data = (char*) malloc((len + 1) * sizeof(char));
ChakraRTInterface::JsCopyString(strValue, data, len + 1, &length);
AssertMsg(len == length, "If you see this message.. There is something seriously wrong. Good Luck!");
*(data + len) = char(0);
// Assume ascii characters
data = (char*)malloc((strLen + 1) * sizeof(char));
errorCode = ChakraRTInterface::JsCopyString(strValue, data, strLen, &writtenLen, &actualLen);
if (errorCode == JsNoError)
{
// If non-ascii, take slow path
if (writtenLen != actualLen)
{
free(data);
data = (char*)malloc((actualLen + 1) * sizeof(char));

errorCode = ChakraRTInterface::JsCopyString(strValue, data, actualLen + 1, &writtenLen, nullptr);
if (errorCode == JsNoError)
{
AssertMsg(actualLen == writtenLen, "If you see this message.. There is something seriously wrong. Good Luck!");

}
}
}
}
}
if (errorCode == JsNoError)
{
*(data + actualLen) = char(0);
}
return errorCode;
}

Expand Down
17 changes: 14 additions & 3 deletions lib/Jsrt/ChakraCore.h
Original file line number Diff line number Diff line change
Expand Up @@ -286,13 +286,23 @@ CHAKRA_API
/// <para>
/// When size of the `buffer` is unknown,
/// `buffer` argument can be nullptr.
/// In that case, `written` argument will return the length needed.
/// In that case, `actualLength` argument will return the length needed to
/// accomodate all the UTF8 decoded bytes present in `value`.
/// `writtenLength` will only get populated when valid `buffer` is passed as
/// argument.
/// </para>
/// </remarks>
/// <param name="value">JavascriptString value</param>
/// <param name="buffer">Pointer to buffer</param>
/// <param name="bufferSize">Buffer size</param>
/// <param name="written">Total number of characters written</param>
/// <param name="writtenLength">Total number of UTF8 decoded bytes written. This is only
/// populated when passed with non-null `buffer`,else is
/// set to 0.
/// </param>
/// <param name="actualLength">Total number of UTF8 decoded bytes present in `value`.
/// Useful to initialize buffer of appropriate size that
/// can be passed in to this API.
/// </param>
/// <returns>
/// The code <c>JsNoError</c> if the operation succeeded, a failure code otherwise.
/// </returns>
Expand All @@ -301,7 +311,8 @@ CHAKRA_API
_In_ JsValueRef value,
_Out_opt_ char* buffer,
_In_ size_t bufferSize,
_Out_opt_ size_t* written);
_Out_opt_ size_t* writtenLength,
_Out_opt_ size_t* actualLength);

/// <summary>
/// Write string value into Utf16 string buffer
Expand Down
21 changes: 12 additions & 9 deletions lib/Jsrt/Jsrt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4252,7 +4252,8 @@ CHAKRA_API JsCopyString(
_In_ JsValueRef value,
_Out_opt_ char* buffer,
_In_ size_t bufferSize,
_Out_opt_ size_t* length)
_Out_opt_ size_t* writtenLength,
_Out_opt_ size_t* actualLength)
{
PARAM_NOT_NULL(value);
VALIDATE_JSREF(value);
Expand All @@ -4266,14 +4267,12 @@ CHAKRA_API JsCopyString(
}

utf8::WideToNarrow utf8Str(str, strLength);
if (!buffer)
if (actualLength)
{
if (length)
{
*length = utf8Str.Length();
}
*actualLength = utf8Str.Length();
}
else

if (buffer)
{
size_t count = min(bufferSize, utf8Str.Length());
// Try to copy whole characters if buffer size insufficient
Expand All @@ -4284,11 +4283,15 @@ CHAKRA_API JsCopyString(
(LPCUTF8)(const char*)utf8Str, utf8Str.Length(), maxFitChars);

memmove(buffer, utf8Str, sizeof(char) * count);
if (length)
if (writtenLength)
{
*length = count;
*writtenLength = count;
}
}
else if (writtenLength)
{
*writtenLength = 0;
}

return JsNoError;
}
Expand Down
4 changes: 2 additions & 2 deletions test/native-tests/test-c98/sample.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ int main()
// Project script result back to C++.
char *resultSTR = nullptr;
size_t stringLength;
FAIL_CHECK(JsCopyString(resultJSString, nullptr, 0, &stringLength));
FAIL_CHECK(JsCopyString(resultJSString, nullptr, 0, nullptr, &stringLength));
resultSTR = (char*)malloc(stringLength + 1);
FAIL_CHECK(JsCopyString(resultJSString, resultSTR, stringLength + 1, nullptr));
FAIL_CHECK(JsCopyString(resultJSString, resultSTR, stringLength + 1, nullptr, nullptr));
resultSTR[stringLength] = 0;

printf("Result -> %s \n", resultSTR);
Expand Down
4 changes: 2 additions & 2 deletions test/native-tests/test-char/sample.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ int main()
// Project script result back to C++.
char *resultSTR = nullptr;
size_t stringLength;
FAIL_CHECK(JsCopyString(resultJSString, nullptr, 0, &stringLength));
FAIL_CHECK(JsCopyString(resultJSString, nullptr, 0, nullptr, &stringLength));
resultSTR = (char*)malloc(stringLength + 1);
FAIL_CHECK(JsCopyString(resultJSString, resultSTR, stringLength + 1, nullptr));
FAIL_CHECK(JsCopyString(resultJSString, resultSTR, stringLength + 1, nullptr, nullptr));
resultSTR[stringLength] = 0;

printf("Result -> %s \n", resultSTR);
Expand Down
4 changes: 2 additions & 2 deletions test/native-tests/test-char16/sample.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@ int main()
// Project script result back to C++.
char *resultSTR = nullptr;
size_t stringLength;
FAIL_CHECK(JsCopyString(resultJSString, nullptr, 0, &stringLength));
FAIL_CHECK(JsCopyString(resultJSString, nullptr, 0, nullptr, &stringLength));
resultSTR = (char*) malloc(stringLength + 1);
FAIL_CHECK(JsCopyString(resultJSString, resultSTR, stringLength + 1, nullptr));
FAIL_CHECK(JsCopyString(resultJSString, resultSTR, stringLength + 1, nullptr, nullptr));
resultSTR[stringLength] = 0;

printf("Result -> %s \n", resultSTR);
Expand Down
4 changes: 2 additions & 2 deletions test/native-tests/test-python/helloWorld.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,12 @@

stringLength = c_size_t();
# Get buffer size needed for the result string
chakraCore.JsCopyString(resultJSString, 0, 0, byref(stringLength));
chakraCore.JsCopyString(resultJSString, 0, 0, 0, byref(stringLength));

resultSTR = create_string_buffer(stringLength.value + 1); # buffer is big enough to store the result

# Get String from JsValueRef
chakraCore.JsCopyString(resultJSString, byref(resultSTR), stringLength.value + 1, 0);
chakraCore.JsCopyString(resultJSString, byref(resultSTR), stringLength.value + 1, 0, 0);

# Set `null-ending` to the end
resultSTRLastByte = (c_char * stringLength.value).from_address(addressof(resultSTR))
Expand Down
4 changes: 2 additions & 2 deletions test/native-tests/test-shared-basic/sample.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ int main()
// Project script result back to C++.
char *resultSTR = nullptr;
size_t stringLength;
FAIL_CHECK(JsCopyString(resultJSString, nullptr, 0, &stringLength));
FAIL_CHECK(JsCopyString(resultJSString, nullptr, 0, nullptr, &stringLength));
resultSTR = (char*) malloc(stringLength + 1);
FAIL_CHECK(JsCopyString(resultJSString, resultSTR, stringLength + 1, nullptr));
FAIL_CHECK(JsCopyString(resultJSString, resultSTR, stringLength + 1, nullptr, nullptr));
resultSTR[stringLength] = 0;

printf("Result -> %s \n", resultSTR);
Expand Down
4 changes: 2 additions & 2 deletions test/native-tests/test-static-native/sample.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ int main()
// Project script result back to C++.
char *resultSTR = nullptr;
size_t stringLength;
FAIL_CHECK(JsCopyString(resultJSString, nullptr, 0, &stringLength));
FAIL_CHECK(JsCopyString(resultJSString, nullptr, 0, nullptr, &stringLength));
resultSTR = (char*) malloc(stringLength + 1);
FAIL_CHECK(JsCopyString(resultJSString, resultSTR, stringLength + 1, nullptr));
FAIL_CHECK(JsCopyString(resultJSString, resultSTR, stringLength + 1, nullptr, nullptr));
resultSTR[stringLength] = 0;

printf("Result -> %s \n", resultSTR);
Expand Down

0 comments on commit 3c7ecf0

Please sign in to comment.