Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Returning null instead of empty string if a wrapped function returns null pointer #22574

Open
suzukieng opened this issue Sep 16, 2024 · 7 comments

Comments

@suzukieng
Copy link

Hi emscripten folks,

I am trying to wrap a function with the following C signature:

const char* getLastError()

It returns a pointer to zero-terminated C string or the NULL pointer in case no last error exists.

Wrapping the function in emscripten using Module.cwrap('getLastError', 'string', []) yields a function that returns the empty string if the wrapped function returns NULL, which was a bit surprising to me.

What is the rationale for returning the empty string instead of JS null in this case?

Thanks!

@sbc100
Copy link
Collaborator

sbc100 commented Sep 18, 2024

That sounds like the correct behaviour yes. Would you have time to send a PR? I think the code in question is in src/library_ccall.js

@sbc100
Copy link
Collaborator

sbc100 commented Sep 18, 2024

Actually it looks like there is already handling for this case:

var ret = 0;
if (str !== null && str !== undefined && str !== 0) { // null string
// at most 4 bytes per UTF-8 code point, +1 for the trailing '\0'
ret = stringToUTF8OnStack(str);
}
return {{{ to64('ret') }}};

It looks like it should return 0 if getLastError returns NULL/0.

@sbc100
Copy link
Collaborator

sbc100 commented Sep 18, 2024

Sorry thats the handler to argument. The handler for returned strings just calls UTF8ToString which for some reason does, as you say, return empty string instead of null:

if (!ptr) return '';

I wonder how much stuff would break if we changed that to return null instead.

CC @brendandahl @kripken @juj

@suzukieng
Copy link
Author

Thanks for responding so quickly, Sam, I really appreciate it.

I agree that changing the behavior of UTF8ToString is probably not worth the effort considering the possible breakage of downstream code. Rewriting the empty string to null in the return handler would also not make much sense IMHO, since the empty string is a legitimate string to return.

I was primarily wondering if this is a conscious design choice or just something that turned out the way it is for reasons unknown.

@sbc100
Copy link
Collaborator

sbc100 commented Sep 19, 2024

I think this is likely an historcal accident.

@kripken
Copy link
Member

kripken commented Sep 19, 2024

I agree changing this is probably not worth it. I'm not sure why it is the way it is, but likely the early users of the function preferred it that way, or it didn't matter.

@suzukieng suzukieng closed this as not planned Won't fix, can't repro, duplicate, stale Sep 19, 2024
@sbc100
Copy link
Collaborator

sbc100 commented Sep 19, 2024

I'd still like to try and make this change if we can.. not sure if there is way to do it in a backwards compatible way..

@suzukieng suzukieng reopened this Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants