fix stdin and stdout on nodejs #303

Closed
wants to merge 3 commits into
from

Projects

None yet

2 participants

@radare
radare commented Mar 7, 2012

newlines where not honored in stdout and hardcoded appended '\n' was breaking printf("foo");
stdin was just not working when running under nodejs. this way it just works for most cases (non interactive)

@kripken kripken commented on the diff Mar 8, 2012
src/library.js
@@ -313,7 +313,9 @@ LibraryManager.library = {
if (!input) input = function() {
if (!input.cache || !input.cache.length) {
var result;
- if (typeof window != 'undefined' &&
+ if (ENVIRONMENT_IS_NODE) {
+ result = require("fs").readSync(0, 1024)[0];
@kripken
kripken Mar 8, 2012 Owner

Why is this reading up to 1024 bytes?

@radare
radare Mar 8, 2012

As far as I understand, this function should just read a single char, but reading only one it's not working, reading 1024 is just a quick hack to make emscripten stdio work with node on commandline. I will investigate if there's a better solution for this, but at least reading 1024 works as expected in most interactive prompt use cases (input is bigger it just fails). Do you know a better way to do this?

Thanks

@kripken
kripken Mar 9, 2012 Owner

I don't know node very well, sorry. But I think it is reading up to 1024 chars from looking at their docs. So I suspect this might not work if the input is bigger than that.

@kripken kripken and 1 other commented on an outdated diff Mar 8, 2012
src/library.js
@@ -327,11 +329,10 @@ LibraryManager.library = {
return input.cache.shift();
};
if (!output) output = function(val) {
+ output.buffer.push(String.fromCharCode(val));
@kripken
kripken Mar 8, 2012 Owner

Moving this line up here will append null to the string, when null is passed, which sounds wrong. Why did you need to move it here? Perhaps there is a better fix.

@radare
radare Mar 8, 2012

fixed in head.

@kripken
Owner
kripken commented Jul 13, 2012

Any updates here? Would be great to finish and land this.

@kripken
Owner
kripken commented Jan 8, 2013

Would be great to see this finished, but no activity for 6 months, so closing this. Please reopen if you want to pick it up.

@kripken kripken closed this Jan 8, 2013
@shlomif shlomif added a commit to shlomif/emscripten that referenced this pull request May 7, 2013
@shlomif shlomif Forward ported and merged Pull request #303. ee909c5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment