Repository navigation
Documentation mismatch for napi_get_value_string_* functions #14398
Description
napi_get_value_string_* say e.g.:
[out] result: Number of bytes copied into the buffer including the null. terminator.
However, the actual implementation in Node returns the number of bytes excluding the null terminator (so for an empty string, it will return 0 and not 1).
Not sure if it's implementation or documentation issue, or I was just confused by the wording. cc @nodejs/n-api
Activity
jasongin commentedon Jul 20, 2017
It's a doc issue. I think the doc was originally written when there were separate APIs for getting the string length vs string contents, and then it was not updated correctly when the APIs were redesigned.
The string APIs now are intentionally designed so they can be used to just get the string length (not including null terminator) by passing in a null buffer. So even when you do provide a buffer, the returned value, which is the number of characters copied, does not include the null terminator for consistency.
RReverser commentedon Jul 20, 2017
Yup, I agree it's more convenient that way, just wanted to know whether it's docs or implementation that got out of sync.
By the way, is there a specific reason this API is not aligned with
napi_get_cb_info? In the latter, you can pass initial count and retrieve actual one via the same pointer tosize_t, while string functions accept these as separate arguments.Given what you said, I suspect the reason was exactly that one referred to initial length excluding null, while the other returned total size of written bytes, but now that in/out have the same meaning, would it be reasonable to align these APIs for consistency? (I can raise a separate issue for that)
jasongin commentedon Jul 20, 2017
The
bufsizeinput parameter is the size of the buffer, which must include space for the null terminator. So I'm not sure it would make sense to combine that with the output parameter.RReverser commentedon Jul 21, 2017
Makes sense I guess...
taveras commentedon Jul 28, 2017
hello! i'm a first-time contributor, and it seems that the issue highlights a change needed in the
following documentation:
doc/api/n-api.mdis it okay if I open a PR for this change?
gabrielschulhof commentedon Jul 28, 2017
@taveras Absolutely :)
doc: fix doc for napi_get_value_string_utf8
doc: fix doc for napi_get_value_string_utf8
doc: fix doc for napi_get_value_string_utf8
doc: fix doc for napi_get_value_string_utf8
doc: fix doc for napi_get_value_string_utf8
doc: fix doc for napi_get_value_string_utf8