-
Notifications
You must be signed in to change notification settings - Fork 683
Use 'ECMA_VALUE_CAN_STORE_UINTPTR_VALUE_DIRECTLY' instead of comparis… #1014
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use 'ECMA_VALUE_CAN_STORE_UINTPTR_VALUE_DIRECTLY' instead of comparis… #1014
Conversation
} | ||
ecma_dealloc_external_pointer (handler_p); | ||
|
||
#endif /* ECMA_VALUE_CAN_STORE_UINTPTR_VALUE_DIRECTLY */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/* !ECMA_VALUE_CAN_STORE_UINTPTR_VALUE_DIRECTLY */
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I've updated this comment.
6198970
to
e00c373
Compare
if (sizeof (ecma_external_pointer_t) == sizeof (uint32_t)) | ||
#ifdef ECMA_VALUE_CAN_STORE_UINTPTR_VALUE_DIRECTLY | ||
|
||
ECMA_PROPERTY_VALUE_PTR (prop_p)->value = (uint32_t) ptr_value; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think ecma_value_t would be a better (future proof) for casting instead of uint32_t.
e00c373
to
bc17527
Compare
Thanks for the comments, I've updated this patch. |
@@ -151,17 +153,15 @@ ecma_free_external_pointer_in_property (ecma_property_t *prop_p) /**< internal p | |||
|| ECMA_PROPERTY_GET_INTERNAL_PROPERTY_TYPE (prop_p) == ECMA_INTERNAL_PROPERTY_NATIVE_HANDLE | |||
|| ECMA_PROPERTY_GET_INTERNAL_PROPERTY_TYPE (prop_p) == ECMA_INTERNAL_PROPERTY_FREE_CALLBACK); | |||
|
|||
if (sizeof (ecma_external_pointer_t) == sizeof (uint32_t)) | |||
{ | |||
/* no additional memory was allocated for the pointer storage */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should keep this comment
bc17527
to
80e9fe4
Compare
LGTM |
|
||
#else /* !ECMA_VALUE_CAN_STORE_UINTPTR_VALUE_DIRECTLY */ | ||
|
||
ecma_external_pointer_t *handler_p = ECMA_GET_NON_NULL_POINTER (ecma_external_pointer_t, | ||
ECMA_PROPERTY_VALUE_PTR (prop_p)->value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is still misaligned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, I've overlooked this, thanks.
80e9fe4
to
4f48cee
Compare
LGTM |
…on with 'sizeof' JerryScript-DCO-1.0-Signed-off-by: Robert Sipka rsipka.uszeged@partner.samsung.com
4f48cee
to
9ee4379
Compare
rebased with master |
…on with 'sizeof'
JerryScript-DCO-1.0-Signed-off-by: Robert Sipka rsipka.uszeged@partner.samsung.com