emscripten icon indicating copy to clipboard operation
emscripten copied to clipboard

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

Open suzukieng opened this issue 1 year ago • 7 comments

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!

suzukieng avatar Sep 16 '24 12:09 suzukieng

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 avatar Sep 18 '24 23:09 sbc100

Actually it looks like there is already handling for this case: https://github.com/emscripten-core/emscripten/blob/645750cdb7a8cf2a9210f328ec2c0e5a3b1126e7/src/library_ccall.js#L33-L38

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

sbc100 avatar Sep 18 '24 23:09 sbc100

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: https://github.com/emscripten-core/emscripten/blob/645750cdb7a8cf2a9210f328ec2c0e5a3b1126e7/src/library_strings.js#L126

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

CC @brendandahl @kripken @juj

sbc100 avatar Sep 18 '24 23:09 sbc100

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.

suzukieng avatar Sep 19 '24 07:09 suzukieng

I think this is likely an historcal accident.

sbc100 avatar Sep 19 '24 13:09 sbc100

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.

kripken avatar Sep 19 '24 16:09 kripken

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..

sbc100 avatar Sep 19 '24 17:09 sbc100

I did some investigation and I'm not sure there is much we can win here. Do you have a use case where you need to distinguish between empty JS string and null?

sbc100 avatar Feb 25 '25 00:02 sbc100

Closing for now, feel free to re-open.

sbc100 avatar Feb 25 '25 00:02 sbc100