n-api: Add optional length to functions accepting const char *

[digitalinfinity]

• Version: 8
• Platform: all
• Subsystem: n-api

Migrating nodejs/abi-stable-node#254 opened by @RReverser to this repo for better visibility:

As discussed in the last meeting, zero-terminated const char*, while popular in C land, might be problematic for consumers in other languages having different string representation (for example, Rust with non-zero-terminated string slices represented as data + length).

Some N-API functions already accept optional length that, when set to -1, would mean a length calculated from zero-terminated string and explicit byte length otherwise.

It's worth to go through a list of functions that accept const char* and add length parameter where it's missing, especially for functions that don't provide variants with napi_value as a string, namely:

• napi_create_function
• napi_define_class
• napi_module_register -> napi_module struct

[jasongin]

The following APIs would be affected by this change:

• napi_module_register() (napi_module struct)
• napi_fatal_error()
• napi_create_function()
• napi_define_class()
• napi_create_async_work() (PR src,async_hooks,n-api: refactor async callback handling #14697)
• napi_async_init() (PR n-api: Context for custom async operations #15189)

These APIs would (presumably) not be changed, because they have functionally equivalent alternatives that take napi_value:

• napi_get_named_property() -> napi_get_property()
• napi_set_named_property() -> napi_set_property()
• napi_has_named_property() -> napi_has_property()
• napi_throw_error() -> napi_create_error() + napi_throw()
• napi_throw_type_error() -> napi_create_type_error() + napi_throw()
• napi_throw_range_error() -> napi_create_range_error() + napi_throw()

The char* parameters in these APIs already have corresponding size_t length parameters:

• napi_create_string_latin1()
• napi_create_string_utf8()
• napi_create_string_utf16()
• napi_get_value_string_latin1()
• napi_get_value_string_utf8()
• napi_get_value_string_utf16()

Also, the napi_extended_error_info struct returned by napi_get_last_error_info() has a char* member, but since it is only consumed by the caller (never provided) I assume it would be easy enough to determine the length to the null-terminator using the equivalent of strlen().

[sampsongao]

#15343
Not sure what should I do in the napi_fatal_error. node::FatalError doesn't have alternative of accepting string length. Should I create a new zero-terminated string before pass them to node::FatalError?

[mhdawson]

Discussed with @sampsongao will likely just use stack allocated string to add null terminator if needed.

[digitalinfinity]

@mhdawson this can be closed now that #15343 has landed? Or are we waiting on #15508 too?

[bnoordhuis]

Both PRs have landed so it's looks like this can be closed out now.