GNOME.org
 

[gjs/globals2: 2/4] jsapi-util: Use string vector in gjs_define_string_array

commit b2792f610d056b1c621f576683a113ceee034f21
Author: Philip Chimento <philip chimento gmail com>
Date:   Sat May 23 11:31:11 2020 -0700

    jsapi-util: Use string vector in gjs_define_string_array
    
    This makes the code cleaner and takes care of memory management. We
    propagate this API change back to all callers as far as possible, as
    well. (gjs_context_define_string_array() is a public C API, so its
    signature can't be changed; and GjsContextPrivate::m_search_path must
    be a POD type because the GObject property setter sets that memory
    location before the GjsContextPrivate constructor runs.
    
    Co-authored-by: Evan Welsh <noreply evanwelsh com>

 gjs/context.cpp    |  17 ++++++--
 gjs/importer.cpp   | 119 +++++++++++++++++++----------------------------------
 gjs/importer.h     |   7 +++-
 gjs/jsapi-util.cpp |  37 ++++++-----------
 gjs/jsapi-util.h   |  16 ++++---
 5 files changed, 80 insertions(+), 116 deletions(-)
---
diff --git a/gjs/context.cpp b/gjs/context.cpp
index 4b7fe238..5f8e042a 100644
--- a/gjs/context.cpp
+++ b/gjs/context.cpp
@@ -37,6 +37,7 @@
 #include <string>  // for u16string
 #include <unordered_map>
 #include <utility>  // for move
+#include <vector>
 
 #include <gio/gio.h>
 #include <girepository.h>
@@ -509,8 +510,10 @@ GjsContextPrivate::GjsContextPrivate(JSContext* cx, GjsContext* public_context)
         g_error("Failed to initialize global strings");
     }
 
-    JS::RootedObject importer(m_cx,
-                              gjs_create_root_importer(m_cx, m_search_path));
+    std::vector<std::string> paths;
+    if (m_search_path)
+        paths = {m_search_path, m_search_path + g_strv_length(m_search_path)};
+    JS::RootedObject importer(m_cx, gjs_create_root_importer(m_cx, paths));
     if (!importer) {
         gjs_log_exception(cx);
         g_error("Failed to create root importer");
@@ -1121,10 +1124,16 @@ gjs_context_define_string_array(GjsContext  *js_context,
 
     JSAutoRealm ar(gjs->context(), gjs->global());
 
+    std::vector<std::string> strings;
+    if (array_values) {
+        if (array_length < 0)
+            array_length = g_strv_length(const_cast<char**>(array_values));
+        strings = {array_values, array_values + array_length};
+    }
+
     JS::RootedObject global_root(gjs->context(), gjs->global());
     if (!gjs_define_string_array(gjs->context(), global_root, array_name,
-                                 array_length, array_values,
-                                 JSPROP_READONLY | JSPROP_PERMANENT)) {
+                                 strings, JSPROP_READONLY | JSPROP_PERMANENT)) {
         gjs_log_exception(gjs->context());
         g_set_error(error,
                     GJS_ERROR,
diff --git a/gjs/importer.cpp b/gjs/importer.cpp
index 53b4c0b3..d294301c 100644
--- a/gjs/importer.cpp
+++ b/gjs/importer.cpp
@@ -31,6 +31,7 @@
 #endif
 
 #include <memory>   // for allocator_traits<>::value_type
+#include <string>
 #include <utility>  // for move
 #include <vector>   // for vector
 
@@ -66,8 +67,6 @@
 
 #define MODULE_INIT_FILENAME "__init__.js"
 
-static char **gjs_search_path = NULL;
-
 typedef struct {
     bool is_root;
 } Importer;
@@ -77,8 +76,8 @@ extern const JSClass gjs_importer_class;
 GJS_DEFINE_PRIV_FROM_JS(Importer, gjs_importer_class)
 
 GJS_JSAPI_RETURN_CONVENTION
-static JSObject *gjs_define_importer(JSContext *, JS::HandleObject,
-    const char *, const char * const *, bool);
+static JSObject* gjs_define_importer(JSContext*, JS::HandleObject, const char*,
+                                     const std::vector<std::string>&, bool);
 
 GJS_JSAPI_RETURN_CONVENTION
 static bool
@@ -205,12 +204,9 @@ define_meta_properties(JSContext       *context,
 }
 
 GJS_JSAPI_RETURN_CONVENTION
-static bool
-import_directory(JSContext          *context,
-                 JS::HandleObject    obj,
-                 const char         *name,
-                 const char * const *full_paths)
-{
+static bool import_directory(JSContext* context, JS::HandleObject obj,
+                             const char* name,
+                             const std::vector<std::string>& full_paths) {
     gjs_debug(GJS_DEBUG_IMPORTER,
               "Importing directory '%s'",
               name);
@@ -516,7 +512,7 @@ static bool do_import(JSContext* context, JS::HandleObject obj, Importer* priv,
     }
 
     GjsAutoChar filename = g_strdup_printf("%s.js", name.get());
-    std::vector<GjsAutoChar> directories;
+    std::vector<std::string> directories;
     JS::RootedValue elem(context);
     JS::RootedString str(context);
 
@@ -563,7 +559,7 @@ static bool do_import(JSContext* context, JS::HandleObject obj, Importer* priv,
             gjs_debug(GJS_DEBUG_IMPORTER,
                       "Adding directory '%s' to child importer '%s'",
                       full_path.get(), name.get());
-            directories.push_back(std::move(full_path));
+            directories.push_back(full_path.get());
         }
 
         /* If we just added to directories, we know we don't need to
@@ -600,14 +596,7 @@ static bool do_import(JSContext* context, JS::HandleObject obj, Importer* priv,
     }
 
     if (!directories.empty()) {
-        /* NULL-terminate the char** */
-        const char **full_paths = g_new0(const char *, directories.size() + 1);
-        for (size_t ix = 0; ix < directories.size(); ix++)
-            full_paths[ix] = directories[ix].get();
-
-        bool result = import_directory(context, obj, name.get(), full_paths);
-        g_free(full_paths);
-        if (!result)
+        if (!import_directory(context, obj, name.get(), directories))
             return false;
 
         gjs_debug(GJS_DEBUG_IMPORTER, "successfully imported directory '%s'",
@@ -864,19 +853,17 @@ importer_new(JSContext *context,
 }
 
 GJS_USE
-static const char* const* gjs_get_search_path(void) {
-    char **search_path;
+static const std::vector<std::string>& gjs_get_search_path(void) {
+    static std::vector<std::string> gjs_search_path;
+    static bool search_path_initialized = false;
 
     /* not thread safe */
 
-    if (!gjs_search_path) {
+    if (!search_path_initialized) {
         const char* const* system_data_dirs;
         const char *envstr;
-        GPtrArray *path;
         gsize i;
 
-        path = g_ptr_array_new();
-
         /* in order of priority */
 
         /* $GJS_PATH */
@@ -885,74 +872,58 @@ static const char* const* gjs_get_search_path(void) {
             char **dirs, **d;
             dirs = g_strsplit(envstr, G_SEARCHPATH_SEPARATOR_S, 0);
             for (d = dirs; *d != NULL; d++)
-                g_ptr_array_add(path, *d);
+                gjs_search_path.push_back(*d);
             /* we assume the array and strings are allocated separately */
             g_free(dirs);
         }
 
-        g_ptr_array_add(path,
-                        g_strdup("resource:///org/gnome/gjs/modules/script/"));
-        g_ptr_array_add(path,
-                        g_strdup("resource:///org/gnome/gjs/modules/core/"));
+        gjs_search_path.push_back("resource:///org/gnome/gjs/modules/script/");
+        gjs_search_path.push_back("resource:///org/gnome/gjs/modules/core/");
 
         /* $XDG_DATA_DIRS /gjs-1.0 */
         system_data_dirs = g_get_system_data_dirs();
         for (i = 0; system_data_dirs[i] != NULL; ++i) {
-            char *s;
-            s = g_build_filename(system_data_dirs[i], "gjs-1.0", NULL);
-            g_ptr_array_add(path, s);
+            GjsAutoChar s =
+                g_build_filename(system_data_dirs[i], "gjs-1.0", nullptr);
+            gjs_search_path.push_back(s.get());
         }
 
         /* ${datadir}/share/gjs-1.0 */
 #ifdef G_OS_WIN32
         extern HMODULE gjs_dll;
         char *basedir = g_win32_get_package_installation_directory_of_module (gjs_dll);
-        char *gjs_data_dir = g_build_filename (basedir, "share", "gjs-1.0", NULL);
-        g_ptr_array_add(path, g_strdup(gjs_data_dir));
-        g_free (gjs_data_dir);
+        GjsAutoChar gjs_data_dir =
+            g_build_filename(basedir, "share", "gjs-1.0", nullptr);
+        gjs_search_path.push_back(gjs_data_dir.get());
         g_free (basedir);
 #else
-        g_ptr_array_add(path, g_strdup(GJS_JS_DIR));
+        gjs_search_path.push_back(GJS_JS_DIR);
 #endif
-
-        g_ptr_array_add(path, NULL);
-
-        search_path = (char**)g_ptr_array_free(path, false);
-
-        gjs_search_path = search_path;
-    } else {
-        search_path = gjs_search_path;
     }
 
-    return const_cast<const char* const*>(search_path);
+    return gjs_search_path;
 }
 
 GJS_JSAPI_RETURN_CONVENTION
-static JSObject*
-gjs_create_importer(JSContext          *context,
-                    const char         *importer_name,
-                    const char * const *initial_search_path,
-                    bool                add_standard_search_path,
-                    bool                is_root,
-                    JS::HandleObject    in_object)
-{
-    char **paths[2] = {0};
-
-    paths[0] = (char**)initial_search_path;
+static JSObject* gjs_create_importer(
+    JSContext* context, const char* importer_name,
+    const std::vector<std::string>& initial_search_path,
+    bool add_standard_search_path, bool is_root, JS::HandleObject in_object) {
+    std::vector<std::string> search_paths = initial_search_path;
     if (add_standard_search_path) {
         /* Stick the "standard" shared search path after the provided one. */
-        paths[1] = (char**)gjs_get_search_path();
+        const std::vector<std::string>& gjs_search_path = gjs_get_search_path();
+        search_paths.insert(search_paths.end(), gjs_search_path.begin(),
+                            gjs_search_path.end());
     }
 
-    GjsAutoStrv search_path = gjs_g_strv_concat(paths, 2);
-
     JS::RootedObject importer(context, importer_new(context, is_root));
 
     /* API users can replace this property from JS, is the idea */
-    if (!gjs_define_string_array(context, importer,
-                                 "searchPath", -1, search_path.as<const char *>(),
-                                 /* settable (no READONLY) but not deleteable (PERMANENT) */
-                                 JSPROP_PERMANENT | JSPROP_RESOLVING))
+    if (!gjs_define_string_array(
+            context, importer, "searchPath", search_paths,
+            /* settable (no READONLY) but not deleteable (PERMANENT) */
+            JSPROP_PERMANENT | JSPROP_RESOLVING))
         return nullptr;
 
     if (!define_meta_properties(context, importer, NULL, importer_name, in_object))
@@ -962,14 +933,10 @@ gjs_create_importer(JSContext          *context,
 }
 
 GJS_JSAPI_RETURN_CONVENTION
-static JSObject *
-gjs_define_importer(JSContext          *context,
-                    JS::HandleObject    in_object,
-                    const char         *importer_name,
-                    const char * const *initial_search_path,
-                    bool                add_standard_search_path)
-
-{
+static JSObject* gjs_define_importer(
+    JSContext* context, JS::HandleObject in_object, const char* importer_name,
+    const std::vector<std::string>& initial_search_path,
+    bool add_standard_search_path) {
     JS::RootedObject importer(context,
         gjs_create_importer(context, importer_name, initial_search_path,
                             add_standard_search_path, false, in_object));
@@ -985,9 +952,7 @@ gjs_define_importer(JSContext          *context,
     return importer;
 }
 
-JSObject *
-gjs_create_root_importer(JSContext          *cx,
-                         const char * const *search_path)
-{
+JSObject* gjs_create_root_importer(
+    JSContext* cx, const std::vector<std::string>& search_path) {
     return gjs_create_importer(cx, "imports", search_path, true, true, nullptr);
 }
diff --git a/gjs/importer.h b/gjs/importer.h
index a0ce8f48..81c6e7dd 100644
--- a/gjs/importer.h
+++ b/gjs/importer.h
@@ -26,13 +26,16 @@
 
 #include <config.h>
 
+#include <string>
+#include <vector>
+
 #include <js/TypeDecls.h>
 
 #include "gjs/macros.h"
 
 GJS_JSAPI_RETURN_CONVENTION
-JSObject *gjs_create_root_importer(JSContext          *cx,
-                                   const char * const *search_path);
+JSObject* gjs_create_root_importer(JSContext* cx,
+                                   const std::vector<std::string>& search_path);
 
 GJS_JSAPI_RETURN_CONVENTION
 bool gjs_import_native_module(JSContext       *cx,
diff --git a/gjs/jsapi-util.cpp b/gjs/jsapi-util.cpp
index 531b1d49..4d7193b2 100644
--- a/gjs/jsapi-util.cpp
+++ b/gjs/jsapi-util.cpp
@@ -34,7 +34,9 @@
 
 #include <codecvt>  // for codecvt_utf8_utf16
 #include <locale>   // for wstring_convert
+#include <string>
 #include <utility>  // for move
+#include <vector>
 
 #include <js/CallArgs.h>
 #include <js/CharacterEncoding.h>
@@ -219,24 +221,16 @@ gjs_throw_abstract_constructor_error(JSContext    *context,
     gjs_throw(context, "You cannot construct new instances of '%s'", name);
 }
 
-JSObject *
-gjs_build_string_array(JSContext   *context,
-                       gssize       array_length,
-                       char       **array_values)
-{
-    int i;
-
-    if (array_length == -1)
-        array_length = g_strv_length(array_values);
-
+JSObject* gjs_build_string_array(JSContext* context,
+                                 const std::vector<std::string>& strings) {
     JS::RootedValueVector elems(context);
-    if (!elems.reserve(array_length)) {
+    if (!elems.reserve(strings.size())) {
         JS_ReportOutOfMemory(context);
         return nullptr;
     }
 
-    for (i = 0; i < array_length; ++i) {
-        JS::ConstUTF8CharsZ chars(array_values[i], strlen(array_values[i]));
+    for (const std::string& string : strings) {
+        JS::ConstUTF8CharsZ chars(string.c_str(), string.size());
         JS::RootedValue element(context,
             JS::StringValue(JS_NewStringCopyUTF8Z(context, chars)));
         elems.infallibleAppend(element);
@@ -245,17 +239,12 @@ gjs_build_string_array(JSContext   *context,
     return JS_NewArrayObject(context, elems);
 }
 
-JSObject*
-gjs_define_string_array(JSContext       *context,
-                        JS::HandleObject in_object,
-                        const char      *array_name,
-                        ssize_t          array_length,
-                        const char     **array_values,
-                        unsigned         attrs)
-{
-    JS::RootedObject array(context,
-        gjs_build_string_array(context, array_length, (char **) array_values));
-
+JSObject* gjs_define_string_array(JSContext* context,
+                                  JS::HandleObject in_object,
+                                  const char* array_name,
+                                  const std::vector<std::string>& strings,
+                                  unsigned attrs) {
+    JS::RootedObject array(context, gjs_build_string_array(context, strings));
     if (!array)
         return nullptr;
 
diff --git a/gjs/jsapi-util.h b/gjs/jsapi-util.h
index 62ecf23c..a4bee3bc 100644
--- a/gjs/jsapi-util.h
+++ b/gjs/jsapi-util.h
@@ -32,6 +32,7 @@
 
 #include <memory>  // for unique_ptr
 #include <string>  // for string, u16string
+#include <vector>
 
 #include <girepository.h>
 #include <glib-object.h>
@@ -217,17 +218,14 @@ void gjs_throw_abstract_constructor_error(JSContext    *context,
                                           JS::CallArgs& args);
 
 GJS_JSAPI_RETURN_CONVENTION
-JSObject*   gjs_build_string_array           (JSContext       *context,
-                                              gssize           array_length,
-                                              char           **array_values);
+JSObject* gjs_build_string_array(JSContext* cx,
+                                 const std::vector<std::string>& strings);
 
 GJS_JSAPI_RETURN_CONVENTION
-JSObject *gjs_define_string_array(JSContext       *context,
-                                  JS::HandleObject obj,
-                                  const char      *array_name,
-                                  ssize_t          array_length,
-                                  const char     **array_values,
-                                  unsigned         attrs);
+JSObject* gjs_define_string_array(JSContext* cx, JS::HandleObject obj,
+                                  const char* array_name,
+                                  const std::vector<std::string>& strings,
+                                  unsigned attrs);
 
 void        gjs_throw                        (JSContext       *context,
                                               const char      *format,
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]