Fix size types #943
Fix size types #943
Conversation
|
Note: this PR does not only solve the related issue but does a lot more. The reason is that the issue pointed at a problem (incorrect or illogical use of various |
| * @return number of bytes, actually copied to the buffer - if string's content was copied successfully; | ||
| * otherwise (in case size of buffer is insufficient) - negative number, which is calculated | ||
| * as negation of buffer size, that is required to hold the string's content. | ||
| * @return number of bytes, actually copied to the buffer. |
There was a problem hiding this comment.
What will happen if the buffer is too small?
There was a problem hiding this comment.
Well, we fail with an assert (in debug). That's aligned with other similar functions (e.g., ecma_uint32_to_utf8_string).
Actually, at almost all call sites, we rely on the buffer being preallocated to the exact size of the string. Perhaps the only counter example is ecma_compare_ecma_strings_relational, where a buffer of fixed size is available and there was a logic to decide whether to used that fixed buffer or allocate a new one, and that logic really relied on a negative return value from ecma_string_to_utf8_string, but it was to rewrite to use ecma_string_get_size instead.
|
This might take effect on performance too. @bzsolt, could you measure it on RPi2? |
|
RPi 2 results Binary sizes (bytes): 187,160 -> 187,164 |
|
@bzsolt, thanks. @akiss77, LGTM. Please squash before merge. |
|
Honestly, I've expected both size and performance gain. Two dead functons have been eliminated, and Anyway, the compiler error is fixed at least, as well as abuse of the |
|
I am happy you spent time on checking this. Thank you. |
E.g., * `ssize_t` was used where `lit_utf8_size_t` or `jerry_api_size_t` would have been correct, * `lit_utf8_size_t` was used where `ecma_length_t` would have been correct. Note, the patch also includes internal and public API changes: * `ecma_string_to_utf8_string` does not return negative value if output buffer is not large enough to contain the string; the buffer is expected to be large enough. (`ecma_string_get_size` can be used to retrieve the required size.) * `jerry_api_string_to_char_buffer` adapts the same logic (and `jerry_api_get_string_size` can be used to determine the required size of the buffer). Related issue: jerryscript-project#942 JerryScript-DCO-1.0-Signed-off-by: Akos Kiss akiss@inf.u-szeged.hu
akosthekiss
force-pushed
the
fix-size-types
branch
from
5826ce2 to
25b0750
Compare
Related issue #942