Minor module-node fixes

* Fix typo in type name (duk_int -> duk_int_t)

* Fix value stack index usage for safe call, was using positive indices

* Add exitcode to test.c so that 'make test' errors out

* Add exitcode to other extras too where applicable

extras/console/test.c

@@ -5,6 +5,7 @@
 int main(int argc, char *argv[]) {
 	duk_context *ctx;
 	int i;
+	int exitcode = 0;
 
 	ctx = duk_create_heap_default();
 	if (!ctx) {
@@ -16,12 +17,14 @@ int main(int argc, char *argv[]) {
 
 	for (i = 1; i < argc; i++) {
 		printf("Evaling: %s\n", argv[i]);
-		(void) duk_peval_string(ctx, argv[i]);
+		if (duk_peval_string(ctx, argv[i]) != 0) {
+			exitcode = 1;
+		}
 		printf("--> %s\n", duk_safe_to_string(ctx, -1));
 		duk_pop(ctx);
 	}
 
 	printf("Done\n");
 	duk_destroy_heap(ctx);
-	return 0;
+	return exitcode;
 }

extras/logging/test.c

@@ -24,6 +24,7 @@ static duk_ret_t init_logging(duk_context *ctx, void *udata) {
 int main(int argc, char *argv[]) {
 	duk_context *ctx;
 	int i;
+	int exitcode = 0;
 
 	ctx = duk_create_heap_default();
 	if (!ctx) {
@@ -38,13 +39,18 @@ int main(int argc, char *argv[]) {
 		printf("Evaling: %s\n", argv[i]);
 		duk_push_string(ctx, argv[i]);
 		duk_push_string(ctx, "evalCodeFileName");  /* for automatic logger name testing */
-		duk_compile(ctx, DUK_COMPILE_EVAL);
-		(void) duk_pcall(ctx, 0);
+		if (duk_pcompile(ctx, DUK_COMPILE_EVAL) != 0) {
+			exitcode = 1;
+		} else {
+			if (duk_pcall(ctx, 0) != 0) {
+				exitcode = 1;
+			}
+		}
 		printf("--> %s\n", duk_safe_to_string(ctx, -1));
 		duk_pop(ctx);
 	}
 
 	printf("Done\n");
 	duk_destroy_heap(ctx);
-	return 0;
+	return exitcode;
 }

extras/module-node/Makefile

@@ -2,15 +2,16 @@
 
 .PHONY: test
 test:
-	gcc -o $@ -I../../src/ -I. ../../src/duktape.c duk_module_node.c test.c -lm
+	gcc -Wall -Wextra -std=c99 -o $@ -I../../src/ -I. ../../src/duktape.c duk_module_node.c test.c -lm
 	@printf '\n'
-	./test 'typeof require("pig") === "string" ? "require() OK" : "require() FAILED";'
-	./test 'require("cow").indexOf("pig") !== -1 ? "nested require() OK" : "nested require() FAILED";'
-	./test 'var ape1 = require("ape"); var ape2 = require("ape"); ape1 === ape2 ? "caching OK" : "caching FAILED";'
-	./test 'var ape1 = require("ape"); var inCache = "ape.js" in require.cache; delete require.cache["ape.js"]; var ape2 = require("ape"); inCache && ape2 !== ape1 ? "require.cache OK" : "require.cache FAILED";'
-	./test 'var ape = require("ape"); typeof ape.module.require === "function" ? "module.require OK" : "module.require FAILED";'
-	./test 'var ape = require("ape"); ape.module.exports === ape ? "module.exports OK" : "module.exports FAILED";'
-	./test 'var ape = require("ape"); ape.module.id === "ape.js" && ape.module.id === ape.module.filename ? "module.id OK" : "module.id FAILED";'
-	./test 'var ape = require("ape"); ape.module.filename === "ape.js" ? "module.filename OK" : "module.filename FAILED";'
-	./test 'var ape = require("ape"); ape.module.loaded === true && ape.wasLoaded === false ? "module.loaded OK" : "module.loaded FAILED";'
-	./test 'var ape = require("ape"); ape.__filename === "ape.js" ? "__filename OK" : "__filename FAILED";'
+	./test 'assert(typeof require("pig") === "string", "basic require()");'
+	./test 'assert(require("cow").indexOf("pig") !== -1, "nested require()");'
+	./test 'var ape1 = require("ape"); var ape2 = require("ape"); assert(ape1 === ape2, "caching");'
+	./test 'var ape1 = require("ape"); var inCache = "ape.js" in require.cache; delete require.cache["ape.js"]; var ape2 = require("ape"); assert(inCache && ape2 !== ape1, "require.cache");'
+	./test 'var ape = require("ape"); assert(typeof ape.module.require === "function", "module.require()");'
+	./test 'var ape = require("ape"); assert(ape.module.exports === ape, "module.exports");'
+	./test 'var ape = require("ape"); assert(ape.module.id === "ape.js" && ape.module.id === ape.module.filename, "module.id");'
+	./test 'var ape = require("ape"); assert(ape.module.filename === "ape.js", "module.filename");'
+	./test 'var ape = require("ape"); assert(ape.module.loaded === true && ape.wasLoaded === false, "module.loaded");'
+	./test 'var ape = require("ape"); assert(ape.__filename === "ape.js", "__filename");'
+	./test 'var badger = require("badger"); assert(badger.foo === 123 && badger.bar === 234, "exports.foo assignment");'

extras/module-node/README.rst

@@ -3,7 +3,7 @@ Node.js-like module loading framework
 =====================================
 
 This directory contains an example module resolution and loading framework and
-``require`` implementation based on the Node.js module system:
+``require()`` implementation based on the Node.js module system:
 
 * https://nodejs.org/api/modules.html
 
@@ -35,9 +35,9 @@ The application needs only to provide the module resolution and loading logic:
   It is possible to replace the callbacks after initialization by setting the
   following internal properties on the global stash:
   
-  - `\xffmodResolve`
+  - ``\xffmodResolve``
   
-  - `\xffmodLoad`
+  - ``\xffmodLoad``
 
 * The resolve callback is a Duktape/C function which takes the string passed
   to ``require()`` and resolves it to a canonical module ID (for Node.js this
@@ -60,11 +60,12 @@ The application needs only to provide the module resolution and loading logic:
 
   If the module ID cannot be resolved, the resolve callback should throw an
   error, which will propagate out of the ``require()`` call.  Note also that
-  when the global ``require`` is called, the parent ID is an empty string.
+  when the global ``require()`` is called, the parent ID is an empty string.
 
 * The load callback is a Duktape/C function which takes the resolved module ID
-  and either returns the Ecmascript source code for the module, or populates
-  ``module.exports`` itself and returns undefined (useful for C modules)::
+  and: (1) returns the Ecmascript source code for the module or ``undefined``
+  if there's no source code, e.g. for pure C modules, (2) can populate
+  ``module.exports`` itself, and (3) can replace ``module.exports``::
 
       duk_ret_t cb_load_module(duk_context *ctx) {
           /*
@@ -80,5 +81,5 @@ The application needs only to provide the module resolution and loading logic:
   As with the resolve callback, the load callback should throw an error if the
   module cannot be loaded for any reason.
 
-* After these steps, ``require`` will be registered to the global object and
+* After these steps, ``require()`` will be registered to the global object and
   the module system is ready to use.

extras/module-node/duk_module_node.c

@@ -61,7 +61,6 @@ static duk_ret_t duk__handle_require(duk_context *ctx) {
 	 */
 
 	const char *id;
-	duk_ret_t nrets;
 	const char *parent_id;
 	duk_idx_t module_idx;
 	duk_idx_t stash_idx;
@@ -73,6 +72,7 @@ static duk_ret_t duk__handle_require(duk_context *ctx) {
 	duk_push_current_function(ctx);
 	(void) duk_get_prop_string(ctx, -1, "\xff" "moduleId");
 	parent_id = duk_require_string(ctx, -1);
+	(void) parent_id;  /* not used directly; suppress warning */
 
 	/* [ id stash require parent_id ] */
 
@@ -116,7 +116,7 @@ static duk_ret_t duk__handle_require(duk_context *ctx) {
 	}
 
 	if (duk_is_string(ctx, -1)) {
-		duk_int ret;
+		duk_int_t ret;
 
 		/* [ ... module source ] */
 
@@ -133,7 +133,7 @@ static duk_ret_t duk__handle_require(duk_context *ctx) {
 		/* nop */
 	} else {
 		duk__del_cached_module(ctx, id);
-		duk_error(ctx, DUK_ERR_API_ERROR, "error in module load callback");
+		duk_error(ctx, DUK_ERR_API_ERROR, "invalid module load callback return value");
 	}
 
 	/* fall through */
@@ -191,7 +191,7 @@ static duk_int_t duk__eval_module_source (duk_context *ctx, void *udata) {
 static duk_int_t duk__eval_module_source (duk_context *ctx) {
 #endif
 	/*
-	 *  Stack: [ module source ]
+	 *  Stack: [ ... module source ]
 	 */
 
 #if DUK_VERSION >= 19999
@@ -203,35 +203,37 @@ static duk_int_t duk__eval_module_source (duk_context *ctx) {
 	 * e.g. Node.js.
 	 */
 	duk_push_string(ctx, "(function main(exports,require,module,__filename,__dirname){");
-	duk_dup(ctx, 1);  /* source */
+	duk_dup(ctx, -2);  /* source */
 	duk_push_string(ctx, "})");
 	duk_concat(ctx, 3);
 
-	/* [ module source func_src ] */
+	/* [ ... module source func_src ] */
 
-	(void) duk_get_prop_string(ctx, 0, "filename");
+	(void) duk_get_prop_string(ctx, -3, "filename");
 	duk_compile(ctx, DUK_COMPILE_EVAL);
 	duk_call(ctx, 0);
 
-	/* [ module source func ] */
+	/* [ ... module source func ] */
 
 	/* call the function wrapper */
-	(void) duk_get_prop_string(ctx, 0, "exports");   /* exports */
-	(void) duk_get_prop_string(ctx, 0, "require");   /* require */
-	duk_dup(ctx, 0);                                 /* module */
-	(void) duk_get_prop_string(ctx, 0, "filename");  /* __filename */
+	(void) duk_get_prop_string(ctx, -3, "exports");   /* exports */
+	(void) duk_get_prop_string(ctx, -4, "require");   /* require */
+	duk_dup(ctx, -5);                                 /* module */
+	(void) duk_get_prop_string(ctx, -6, "filename");  /* __filename */
 	duk_push_undefined(ctx);                          /* __dirname */
 	duk_call(ctx, 5);
 
+	/* [ ... module source result(ignore) ] */
+
 	/* module.loaded = true */
 	duk_push_true(ctx);
-	duk_put_prop_string(ctx, 0, "loaded");
+	duk_put_prop_string(ctx, -4, "loaded");
 
-	/* [ module source retval ] */
+	/* [ ... module source retval ] */
 
 	duk_pop_2(ctx);
 
-	/* [ module ] */
+	/* [ ... module ] */
 
 	return 1;
 }
@@ -243,16 +245,18 @@ void duk_module_node_init(duk_context *ctx) {
 
 	duk_idx_t options_idx;
 
-	duk_require_object_coercible(ctx, -1);
+	duk_require_object_coercible(ctx, -1);  /* error before setting up requireCache */
 	options_idx = duk_require_normalize_index(ctx, -1);
 
-	/* initialize the require cache to a fresh object */
+	/* Initialize the require cache to a fresh object. */
 	duk_push_global_stash(ctx);
 	duk_push_object(ctx);
 	duk_put_prop_string(ctx, -2, "\xff" "requireCache");
 	duk_pop(ctx);
 
-	/* stash callbacks for later use */
+	/* Stash callbacks for later use.  User code can overwrite them later
+	 * on directly by accessing the global stash.
+	 */
 	duk_push_global_stash(ctx);
 	duk_get_prop_string(ctx, options_idx, "resolve");
 	duk_require_function(ctx, -1);

extras/module-node/test.c

@@ -34,8 +34,10 @@ static duk_ret_t cb_load_module(duk_context *ctx) {
 		duk_push_string(ctx, "module.exports = require('pig');");
 	} else if (strcmp(module_id, "ape.js") == 0) {
 		duk_push_string(ctx, "module.exports = { module: module, __filename: __filename, wasLoaded: module.loaded };");
+	} else if (strcmp(module_id, "badger.js") == 0) {
+		duk_push_string(ctx, "exports.foo = 123; exports.bar = 234;");
 	} else {
-		duk_push_string(ctx, "module.exports = undefined;");
+		duk_error(ctx, DUK_ERR_TYPE_ERROR, "cannot find module: %s", module_id);
 	}
 
 	return 1;
@@ -46,15 +48,29 @@ static duk_ret_t handle_print(duk_context *ctx) {
 	return 0;
 }
 
+static duk_ret_t handle_assert(duk_context *ctx) {
+	if (duk_to_boolean(ctx, 0)) {
+		return 0;
+	}
+	duk_error(ctx, DUK_ERR_ERROR, "assertion failed: %s", duk_safe_to_string(ctx, 1));
+	return 0;
+}
+
 int main(int argc, char *argv[]) {
 	duk_context *ctx;
 	int i;
+	int exitcode = 0;
 
 	ctx = duk_create_heap_default();
 	if (!ctx) {
 		return 1;
 	}
 
+	duk_push_c_function(ctx, handle_print, 1);
+	duk_put_global_string(ctx, "print");
+	duk_push_c_function(ctx, handle_assert, 2);
+	duk_put_global_string(ctx, "assert");
+
 	duk_push_object(ctx);
 	duk_push_c_function(ctx, cb_resolve_module, DUK_VARARGS);
 	duk_put_prop_string(ctx, -2, "resolve");
@@ -65,12 +81,19 @@ int main(int argc, char *argv[]) {
 
 	for (i = 1; i < argc; i++) {
 		printf("Evaling: %s\n", argv[i]);
-		(void) duk_peval_string(ctx, argv[i]);
+		if (duk_peval_string(ctx, argv[i]) != 0) {
+			if (duk_get_prop_string(ctx, -1, "stack")) {
+				duk_replace(ctx, -2);
+			} else {
+				duk_pop(ctx);
+			}
+			exitcode = 1;
+		}
 		printf("--> %s\n", duk_safe_to_string(ctx, -1));
 		duk_pop(ctx);
 	}
 
-	printf("Done\n\n");
+	printf("Done\n");
 	duk_destroy_heap(ctx);
-	return 0;
+	return exitcode;
 }

extras/print-alert/test.c

@@ -5,6 +5,7 @@
 int main(int argc, char *argv[]) {
 	duk_context *ctx;
 	int i;
+	int exitcode = 0;
 
 	ctx = duk_create_heap_default();
 	if (!ctx) {
@@ -16,12 +17,14 @@ int main(int argc, char *argv[]) {
 
 	for (i = 1; i < argc; i++) {
 		printf("Evaling: %s\n", argv[i]);
-		(void) duk_peval_string(ctx, argv[i]);
+		if (duk_peval_string(ctx, argv[i]) != 0) {
+			exitcode = 1;
+		}
 		printf("--> %s\n", duk_safe_to_string(ctx, -1));
 		duk_pop(ctx);
 	}
 
 	printf("Done\n");
 	duk_destroy_heap(ctx);
-	return 0;
+	return exitcode;
 }