Small fixes and additional functions#252
Conversation
| JS_ThrowReferenceError(ctx, "shared library modules are not supported yet"); | ||
| return NULL; | ||
| JSModuleDef *m; | ||
| HANDLE hd; | ||
| JSInitModuleFunc *init; | ||
| char *filename; | ||
|
|
||
| if (!strchr(module_name, '/')) { | ||
| /* must add a '/' so that the DLL is not searched in the | ||
| system library paths */ | ||
| filename = js_malloc(ctx, strlen(module_name) + 2 + 1); | ||
| if (!filename) | ||
| return NULL; | ||
| strcpy(filename, "./"); | ||
| strcpy(filename + 2, module_name); | ||
| } else { | ||
| filename = (char *)module_name; | ||
| } | ||
|
|
||
| /* C module */ | ||
| hd = LoadLibrary(filename); | ||
| if (filename != module_name) | ||
| js_free(ctx, filename); | ||
| if (!hd) { | ||
| JS_ThrowReferenceError(ctx, "could not load module filename '%s'('%s') as shared library", | ||
| module_name,filename); | ||
| goto fail; | ||
| } | ||
|
|
||
| init = (JSInitModuleFunc*)GetProcAddress(hd,"js_init_module"); | ||
| if (!init) { | ||
| JS_ThrowReferenceError(ctx, "could not load module filename '%s': js_init_module not found", | ||
| module_name); | ||
| goto fail; | ||
| } | ||
|
|
||
| m = init(ctx, module_name); | ||
| if (!m) { | ||
| JS_ThrowReferenceError(ctx, "could not load module filename '%s': initialization error", | ||
| module_name); | ||
| fail: | ||
| if (hd) | ||
| FreeLibrary(hd); | ||
| return NULL; | ||
| } | ||
| return m; |
There was a problem hiding this comment.
Instead of duplicating the whole function, I would favor using wrappers for the handle declaration, module loading, symbol lookup and module unloading, using macros:
#if defined(_WIN32)
typedef Handle lib_handle;
#define lib_open(name) LoadLibrary(name)
#define lib_sym(hd, name) GetProcAddress(hd, name)
#define lib_close(hd) FreeLibrary(hd)
#define lib_extension ".dll"
#else
typedef void *lib_handle;
#define lib_open(name) dlopen(name, RTLD_NOW | RTLD_LOCAL)
#define lib_sym(hd, name) dlsym(hd, name)
#define lib_close(hd) dlclose(hd)
#define lib_extension ".so"
#endif| JSValue JS_NewStringWLen(JSContext *ctx, size_t buf_len) | ||
| { | ||
| JSString *str; | ||
| if (buf_len <= 0) { | ||
| return JS_AtomToString(ctx, JS_ATOM_empty_string); | ||
| } | ||
| str = js_alloc_string_rt(ctx->rt, buf_len, 0); | ||
| if (unlikely(!str)){ | ||
| JS_ThrowOutOfMemory(ctx); | ||
| return JS_EXCEPTION; | ||
| } | ||
| memset(str->u.str8, 0, buf_len+1); | ||
| return JS_MKPTR(JS_TAG_STRING, str); | ||
| } |
There was a problem hiding this comment.
Instead of a new API, I would modify JS_NewStringLen to accept a null pointer and initialize the allocated string with null bytes using this:
if (buf_len <= 0) {
return JS_AtomToString(ctx, JS_ATOM_empty_string);
}
if (buf == NULL) {
JSString *str = js_alloc_string(ctx, buf_len, 0);
if (!str)
return JS_EXCEPTION;
memset(str->u.str8, buf, buf_len + 1);
return JS_MKPTR(JS_TAG_STRING, str);
}
| uint8_t *JS_ToCStringLenRaw(JSContext *ctx, size_t *plen, JSValueConst val) | ||
| { | ||
| if (JS_VALUE_GET_TAG(val) != JS_TAG_STRING) | ||
| { | ||
| if(plen) *plen = 0; | ||
| return NULL; | ||
| } | ||
| JSString *str = JS_VALUE_GET_STRING(val); | ||
| if(plen) *plen = str->len; | ||
| return str->u.str8; | ||
| } | ||
|
|
There was a problem hiding this comment.
This one has a problem: the caller cannot determine if the string was encoded as 8-bit or 16-bit. Furthermore the name is misleading: the returned pointer is not UTF-8 encoded if the string has non-ASCII characters.
| JSValue JS_NewObjectClass(JSContext *ctx, int class_id) | ||
| JSValue JS_NewObjectClass(JSContext *ctx, JSClassID class_id) |
There was a problem hiding this comment.
The prototype must be changed in quickjs.h too.
|
Hello @zwarrior1, Thank you for contributing. I cannot merge your patch as posted but will apply your patches with attribution after some adjustments. See my comments inline. Chqrlie. |
|
I'd love to see this broken down into smaller, focused PRs since the changes don't depend on each other. |
Of course. But given the change requests, I am going to do this myself. |
3 participants
Hi, I have been using QuickJS as a wrapper for a long time.
these are the fixed I have made.