GNOME.org
 

[gjs: 11/25] js: Change gjs_string_to_utf8() to return JS::UniqueChars

commit 5978920aa4132752928301febc5029d1e6d73ab6
Author: Philip Chimento <philip chimento gmail com>
Date:   Mon May 20 19:23:09 2019 -0700

    js: Change gjs_string_to_utf8() to return JS::UniqueChars
    
    JS_EncodeStringToUTF8() now returns JS::UniqueChars in SpiderMonkey 68,
    so gjs_string_to_utf8() which is a thin wrapper around it might as well
    do that too.
    
    This also fixes a few cases where we created JS::UniqueChars with
    js_strdup(), which doesn't exist anymore.

 gi/arg.cpp                |  4 ++--
 gi/gerror.cpp             |  8 ++++----
 gjs/deprecation.cpp       |  2 +-
 gjs/importer.cpp          |  9 +++++----
 gjs/jsapi-util-args.h     |  5 ++++-
 gjs/jsapi-util-error.cpp  |  2 +-
 gjs/jsapi-util-string.cpp | 23 ++++++++++-------------
 gjs/jsapi-util.cpp        | 41 ++++++++++++++++++++---------------------
 gjs/jsapi-util.h          |  3 +--
 test/gjs-tests.cpp        |  8 +++-----
 10 files changed, 51 insertions(+), 54 deletions(-)
---
diff --git a/gi/arg.cpp b/gi/arg.cpp
index 794ddef7..8919764c 100644
--- a/gi/arg.cpp
+++ b/gi/arg.cpp
@@ -622,8 +622,8 @@ gjs_array_to_strv(JSContext   *context,
             return false;
         }
 
-        JS::UniqueChars tmp_result;
-        if (!gjs_string_to_utf8(context, elem, &tmp_result)) {
+        JS::UniqueChars tmp_result = gjs_string_to_utf8(context, elem);
+        if (!tmp_result) {
             g_strfreev(result);
             return false;
         }
diff --git a/gi/gerror.cpp b/gi/gerror.cpp
index 7015683b..ca45c384 100644
--- a/gi/gerror.cpp
+++ b/gi/gerror.cpp
@@ -463,16 +463,16 @@ gjs_gerror_make_from_error(JSContext       *cx,
     if (!JS_GetPropertyById(cx, obj, atoms.name(), &v_name))
         return nullptr;
 
-    JS::UniqueChars name;
-    if (!gjs_string_to_utf8(cx, v_name, &name))
+    JS::UniqueChars name = gjs_string_to_utf8(cx, v_name);
+    if (!name)
         return nullptr;
 
     JS::RootedValue v_message(cx);
     if (!JS_GetPropertyById(cx, obj, atoms.message(), &v_message))
         return nullptr;
 
-    JS::UniqueChars message;
-    if (!gjs_string_to_utf8(cx, v_message, &message))
+    JS::UniqueChars message = gjs_string_to_utf8(cx, v_message);
+    if (!message)
         return nullptr;
 
     GjsAutoTypeClass<GEnumClass> klass(GJS_TYPE_JS_ERROR);
diff --git a/gjs/deprecation.cpp b/gjs/deprecation.cpp
index b271f334..d2a5caef 100644
--- a/gjs/deprecation.cpp
+++ b/gjs/deprecation.cpp
@@ -74,7 +74,7 @@ struct hash<DeprecationEntry> {
 static std::unordered_set<DeprecationEntry> logged_messages;
 
 GJS_JSAPI_RETURN_CONVENTION
-static char* get_callsite(JSContext* cx) {
+static JS::UniqueChars get_callsite(JSContext* cx) {
     JS::RootedObject stack_frame(cx);
     if (!JS::CaptureCurrentStack(cx, &stack_frame,
                                  JS::StackCapture(JS::MaxFrames(1))) ||
diff --git a/gjs/importer.cpp b/gjs/importer.cpp
index 7e71e727..5d39636f 100644
--- a/gjs/importer.cpp
+++ b/gjs/importer.cpp
@@ -90,8 +90,8 @@ importer_to_string(JSContext *cx,
     if (module_path.isNull()) {
         output = g_strdup_printf("[%s root]", klass->name);
     } else {
-        JS::UniqueChars path;
-        if (!gjs_string_to_utf8(cx, module_path, &path))
+        JS::UniqueChars path = gjs_string_to_utf8(cx, module_path);
+        if (!path)
             return false;
         output = g_strdup_printf("[%s %s]", klass->name, path.get());
     }
@@ -156,8 +156,9 @@ define_meta_properties(JSContext       *context,
         if (parent_module_path.isNull()) {
             module_path_buf = g_strdup(module_name);
         } else {
-            JS::UniqueChars parent_path;
-            if (!gjs_string_to_utf8(context, parent_module_path, &parent_path))
+            JS::UniqueChars parent_path =
+                gjs_string_to_utf8(context, parent_module_path);
+            if (!parent_path)
                 return false;
             module_path_buf = g_strdup_printf("%s.%s", parent_path.get(), module_name);
         }
diff --git a/gjs/jsapi-util-args.h b/gjs/jsapi-util-args.h
index d8bb99ab..f659d119 100644
--- a/gjs/jsapi-util-args.h
+++ b/gjs/jsapi-util-args.h
@@ -29,6 +29,7 @@
 #include <stdint.h>
 
 #include <type_traits>  // for enable_if, is_enum, is_same
+#include <utility>      // for move
 
 #include <glib.h>
 
@@ -88,8 +89,10 @@ static inline void assign(JSContext* cx, char c, bool nullable,
         ref->reset();
         return;
     }
-    if (!gjs_string_to_utf8(cx, value, ref))
+    JS::UniqueChars tmp = gjs_string_to_utf8(cx, value);
+    if (!tmp)
         throw g_strdup("Couldn't convert to string");
+    *ref = std::move(tmp);
 }
 
 GJS_ALWAYS_INLINE
diff --git a/gjs/jsapi-util-error.cpp b/gjs/jsapi-util-error.cpp
index f39ce69d..2de11199 100644
--- a/gjs/jsapi-util-error.cpp
+++ b/gjs/jsapi-util-error.cpp
@@ -211,7 +211,7 @@ gjs_format_stack_trace(JSContext       *cx,
     JS::RootedString stack_trace(cx);
     JS::UniqueChars stack_utf8;
     if (JS::BuildStackString(cx, saved_frame, &stack_trace, 2))
-        stack_utf8.reset(JS_EncodeStringToUTF8(cx, stack_trace));
+        stack_utf8 = JS_EncodeStringToUTF8(cx, stack_trace);
 
     saved_exc.restore();
 
diff --git a/gjs/jsapi-util-string.cpp b/gjs/jsapi-util-string.cpp
index 6674cede..f4eacc01 100644
--- a/gjs/jsapi-util-string.cpp
+++ b/gjs/jsapi-util-string.cpp
@@ -51,7 +51,6 @@ char* gjs_hyphen_to_underscore(const char* str) {
  * gjs_string_to_utf8:
  * @cx: JSContext
  * @value: a JS::Value containing a string
- * @utf8_string_p: return location for a unique JS chars pointer
  *
  * Converts the JSString in @value to UTF-8 and puts it in @utf8_string_p.
  *
@@ -59,17 +58,17 @@ char* gjs_hyphen_to_underscore(const char* str) {
  * typechecks the JS::Value and throws an exception if it's the wrong type.
  * Don't use this function if you already have a JS::RootedString, or if you
  * know the value already holds a string; use JS_EncodeStringToUTF8() instead.
+ *
+ * Returns: Unique UTF8 chars, empty on exception throw.
  */
-bool gjs_string_to_utf8(JSContext* cx, const JS::Value value,
-                        JS::UniqueChars* utf8_string_p) {
+JS::UniqueChars gjs_string_to_utf8(JSContext* cx, const JS::Value value) {
     if (!value.isString()) {
         gjs_throw(cx, "Value is not a string, cannot convert to UTF-8");
-        return false;
+        return nullptr;
     }
 
     JS::RootedString str(cx, value.toString());
-    utf8_string_p->reset(JS_EncodeStringToUTF8(cx, str));
-    return !!*utf8_string_p;
+    return JS_EncodeStringToUTF8(cx, str);
 }
 
 bool
@@ -105,14 +104,12 @@ gjs_string_to_filename(JSContext      *context,
                        GjsAutoChar    *filename_string)
 {
     GError *error;
-    JS::UniqueChars tmp;
 
     /* gjs_string_to_filename verifies that filename_val is a string */
 
-    if (!gjs_string_to_utf8(context, filename_val, &tmp)) {
-        /* exception already set */
+    JS::UniqueChars tmp = gjs_string_to_utf8(context, filename_val);
+    if (!tmp)
         return false;
-    }
 
     error = NULL;
     *filename_string =
@@ -325,7 +322,7 @@ bool gjs_get_string_id(JSContext* cx, jsid id, JS::UniqueChars* name_p) {
     }
 
     JS::RootedString s(cx, JS_FORGET_STRING_FLATNESS(JSID_TO_FLAT_STRING(id)));
-    name_p->reset(JS_EncodeStringToUTF8(cx, s));
+    *name_p = JS_EncodeStringToUTF8(cx, s);
     return !!*name_p;
 }
 
@@ -346,8 +343,8 @@ gjs_unichar_from_string (JSContext *context,
                          JS::Value  value,
                          gunichar  *result)
 {
-    JS::UniqueChars utf8_str;
-    if (gjs_string_to_utf8(context, value, &utf8_str)) {
+    JS::UniqueChars utf8_str = gjs_string_to_utf8(context, value);
+    if (utf8_str) {
         *result = g_utf8_get_char(utf8_str.get());
         return true;
     }
diff --git a/gjs/jsapi-util.cpp b/gjs/jsapi-util.cpp
index 1993307e..161ee4a3 100644
--- a/gjs/jsapi-util.cpp
+++ b/gjs/jsapi-util.cpp
@@ -32,6 +32,7 @@
 
 #include <codecvt>  // for codecvt_utf8_utf16
 #include <locale>   // for wstring_convert
+#include <utility>  // for move
 
 #include "gjs/jsapi-wrapper.h"
 
@@ -124,15 +125,19 @@ gjs_object_require_property(JSContext       *cx,
     return false;
 }
 
-/* Converts JS string value to UTF-8 string. value must be freed with JS_free. */
+/* Converts JS string value to UTF-8 string. */
 bool gjs_object_require_property(JSContext* cx, JS::HandleObject obj,
                                  const char* description,
                                  JS::HandleId property_name,
                                  JS::UniqueChars* value) {
     JS::RootedValue prop_value(cx);
-    if (JS_GetPropertyById(cx, obj, property_name, &prop_value) &&
-        gjs_string_to_utf8(cx, prop_value, value))
-        return true;
+    if (JS_GetPropertyById(cx, obj, property_name, &prop_value)) {
+        JS::UniqueChars tmp = gjs_string_to_utf8(cx, prop_value);
+        if (tmp) {
+            *value = std::move(tmp);
+            return true;
+        }
+    }
 
     throw_property_lookup_error(cx, obj, description, property_name,
                                 "it was not a valid string");
@@ -339,9 +344,6 @@ char*
 gjs_value_debug_string(JSContext      *context,
                        JS::HandleValue value)
 {
-    char *bytes;
-    char *debugstr;
-
     /* Special case debug strings for strings */
     if (value.isString()) {
         JS::RootedString str(context, value.toString());
@@ -377,12 +379,8 @@ gjs_value_debug_string(JSContext      *context,
 
     g_assert(str);
 
-    bytes = JS_EncodeStringToUTF8(context, str);
-
-    debugstr = _gjs_g_utf8_make_valid(bytes);
-    JS_free(context, bytes);
-
-    return debugstr;
+    JS::UniqueChars bytes = JS_EncodeStringToUTF8(context, str);
+    return _gjs_g_utf8_make_valid(bytes.get());
 }
 
 bool
@@ -397,7 +395,7 @@ gjs_log_exception_full(JSContext       *context,
     JS::RootedString exc_str(context, JS::ToString(context, exc));
     JS::UniqueChars utf8_exception;
     if (exc_str)
-        utf8_exception.reset(JS_EncodeStringToUTF8(context, exc_str));
+        utf8_exception = JS_EncodeStringToUTF8(context, exc_str);
     if (!utf8_exception)
         JS_ClearPendingException(context);
 
@@ -411,7 +409,7 @@ gjs_log_exception_full(JSContext       *context,
 
     JS::UniqueChars utf8_message;
     if (message)
-        utf8_message.reset(JS_EncodeStringToUTF8(context, message));
+        utf8_message = JS_EncodeStringToUTF8(context, message);
 
     /* We log syntax errors differently, because the stack for those includes
        only the referencing module, but we want to print out the filename and
@@ -429,19 +427,20 @@ gjs_log_exception_full(JSContext       *context,
         JS::UniqueChars utf8_filename;
         if (js_fileName.isString()) {
             JS::RootedString str(context, js_fileName.toString());
-            utf8_filename.reset(JS_EncodeStringToUTF8(context, str));
+            utf8_filename = JS_EncodeStringToUTF8(context, str);
         }
-        if (!utf8_filename)
-            utf8_filename.reset(JS_strdup(context, "unknown"));
 
         lineNumber = js_lineNumber.toInt32();
 
         if (message) {
             g_critical("JS ERROR: %s: %s @ %s:%u", utf8_message.get(),
-                       utf8_exception.get(), utf8_filename.get(), lineNumber);
+                       utf8_exception.get(),
+                       utf8_filename ? utf8_filename.get() : "unknown",
+                       lineNumber);
         } else {
             g_critical("JS ERROR: %s @ %s:%u", utf8_exception.get(),
-                       utf8_filename.get(), lineNumber);
+                       utf8_filename ? utf8_filename.get() : "unknown",
+                       lineNumber);
         }
 
     } else {
@@ -452,7 +451,7 @@ gjs_log_exception_full(JSContext       *context,
             JS_GetPropertyById(context, exc_obj, atoms.stack(), &stack) &&
             stack.isString()) {
             JS::RootedString str(context, stack.toString());
-            utf8_stack.reset(JS_EncodeStringToUTF8(context, str));
+            utf8_stack = JS_EncodeStringToUTF8(context, str);
         }
 
         if (message) {
diff --git a/gjs/jsapi-util.h b/gjs/jsapi-util.h
index f011a395..45d0c945 100644
--- a/gjs/jsapi-util.h
+++ b/gjs/jsapi-util.h
@@ -240,8 +240,7 @@ char *gjs_value_debug_string(JSContext      *context,
 void gjs_warning_reporter(JSContext*, JSErrorReport* report);
 
 GJS_JSAPI_RETURN_CONVENTION
-bool gjs_string_to_utf8(JSContext* cx, const JS::Value string_val,
-                        JS::UniqueChars* utf8_string_p);
+JS::UniqueChars gjs_string_to_utf8(JSContext* cx, const JS::Value string_val);
 GJS_JSAPI_RETURN_CONVENTION
 bool gjs_string_from_utf8(JSContext             *context,
                           const char            *utf8_string,
diff --git a/test/gjs-tests.cpp b/test/gjs-tests.cpp
index 0fa8d75d..3151629e 100644
--- a/test/gjs-tests.cpp
+++ b/test/gjs-tests.cpp
@@ -136,8 +136,8 @@ static void gjstest_test_func_gjs_jsapi_util_string_js_string_utf8(
     g_assert_true(gjs_string_from_utf8(fx->cx, VALID_UTF8_STRING, &js_string));
     g_assert(js_string.isString());
 
-    JS::UniqueChars utf8_result;
-    g_assert(gjs_string_to_utf8(fx->cx, js_string, &utf8_result));
+    JS::UniqueChars utf8_result = gjs_string_to_utf8(fx->cx, js_string);
+    g_assert_nonnull(utf8_result);
     g_assert_cmpstr(VALID_UTF8_STRING, ==, utf8_result.get());
 }
 
@@ -159,9 +159,7 @@ static void gjstest_test_func_gjs_jsapi_util_error_throw(GjsUnitTestFixture* fx,
 
     g_assert(value.isString());
 
-    JS::UniqueChars s;
-    bool ok = gjs_string_to_utf8(fx->cx, value, &s);
-    g_assert_true(ok);
+    JS::UniqueChars s = gjs_string_to_utf8(fx->cx, value);
     g_assert_nonnull(s);
     g_assert_cmpstr(s.get(), ==, "This is an exception 42");
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]