Fix jerry_create_error_from_value #2403
Conversation
| jerry_char_t str_buff2[str_size]; | ||
| jerry_string_to_utf8_char_buffer (str, str_buff2, str_size); | ||
| str_buff2[str_size] = '\0'; | ||
| JERRY_ASSERT (!strcmp ((char *) str_buff, (char *) str_buff2)); |
There was a problem hiding this comment.
I would directly compare to "Pterodactylus", no need to get it from the str value.
| } | ||
|
|
||
| value = jerry_get_value_from_error (value, release); | ||
| release = true; |
There was a problem hiding this comment.
Just a question: is there an existing test for this case?
There was a problem hiding this comment.
No, there isn't. I will add it, too.
| jerry_release_value (num2); | ||
|
|
||
| num = jerry_create_number (3.1415926); | ||
| dNum = jerry_get_number_value (num); |
There was a problem hiding this comment.
Perhaps you can simply use a local variable here:
double test_num = 3.1415926
jerry_value_t num = jerry_create_number (test_num);
jerry_value_t num2 = jerry_create_error_from_value (num, false);
JERRY_ASSERT (jerry_value_is_error (num2));
jerry_release_value (num);
num2 = jerry_get_value_from_error (num2, true);
JERRY_ASSERT (jerry_get_number_value (num2) == test_num);
jerry_release_value (num2);
The other test cases should be updated as well.
There was a problem hiding this comment.
This could be an interesting test case as well:
double test_num = 3.1415926
jerry_value_t num = jerry_create_number (test_num);
jerry_value_t num2 = jerry_create_error_from_value (num, false);
JERRY_ASSERT (jerry_value_is_error (num2));
jerry_release_value (num);
jerry_value_t num3 = jerry_create_error_from_value (num2, false);
JERRY_ASSERT (jerry_value_is_error (num3));
jerry_release_value (num2);
num2 = jerry_get_value_from_error (num3, true);
JERRY_ASSERT (jerry_get_number_value (num3) == test_num);
jerry_release_value (num3);
With both jerry_create_error_from_value (num2, false); and jerry_create_error_from_value (num2, true);
imiklos
force-pushed
the
fix-jerry_create_error
branch
from
3ad813f to
73072b8
Compare
|
@zherczeg I updated the PR with the changes you requested. |
| jerry_string_to_utf8_char_buffer (str, str_buff, str_size); | ||
| str_buff[str_size] = '\0'; | ||
| JERRY_ASSERT (!strcmp ("Pterodactylus", (char *) str_buff)); | ||
| jerry_release_value (str); |
There was a problem hiding this comment.
This part from jerry_size_t str_size = jerry_get_string_size (str); repeated many times. Perhaps we could introduce a helper function for it. Something like a void compare_str_to(ecma_value_t value, const char *str_p, size_t str_len)
| jerry_size_t str_size = jerry_get_string_size (str); | ||
| jerry_char_t str_buff[str_size]; | ||
| jerry_string_to_utf8_char_buffer (str, str_buff, str_size); | ||
| str_buff[str_size] = '\0'; |
There was a problem hiding this comment.
str_buff[str_size] = '\0'; this is a memory overwrite since jerry_char_t str_buff[str_size]; is defined this way. I would recommend to remove str_buff[str_size] = '\0'; and use memcmp() instead of strcmp() below, since memcmp() has a size argument. Furthermore check the string size with a separate assert right after jerry_get_string_size.
You can also add a:
const char *pterodactylus_p = "Pterodactylus";
const size_t pterodactylus_size = strlen(pterodactylus_p);
at the beginning and use these values for assertion checks.
imiklos
force-pushed
the
fix-jerry_create_error
branch
from
73072b8 to
370837c
Compare
|
@zherczeg I updated the PR |
| { | ||
| jerry_size_t size = jerry_get_string_size (value); | ||
| JERRY_ASSERT (str_len == size); | ||
| jerry_char_t str_buff[size]; |
There was a problem hiding this comment.
I just remember that there is a JERRY_VLA macro for defining such arrays to allow windows compatibility.
There was a problem hiding this comment.
Sure, I changed it to JERRY_VLA and updated the PR.
Fixed the release issue, added some test cases for the function JerryScript-DCO-1.0-Signed-off-by: Istvan Miklos imiklos2@inf.u-szeged.hu
imiklos
force-pushed
the
fix-jerry_create_error
branch
from
370837c to
dfb4320
Compare
3 participants
Fixed the release issue, added some test cases for the function
JerryScript-DCO-1.0-Signed-off-by: Istvan Miklos imiklos2@inf.u-szeged.hu