Skip to content

Conversation

@robertsipka
Copy link
Contributor

…on with 'sizeof'

JerryScript-DCO-1.0-Signed-off-by: Robert Sipka rsipka.uszeged@partner.samsung.com

}
ecma_dealloc_external_pointer (handler_p);

#endif /* ECMA_VALUE_CAN_STORE_UINTPTR_VALUE_DIRECTLY */
Copy link
Contributor

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 */

Copy link
Contributor Author

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.

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;
Copy link
Member

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.

@robertsipka
Copy link
Contributor Author

Thanks for the comments, I've updated this patch.


if (sizeof (ecma_external_pointer_t) == sizeof (uint32_t))
{
/* no additional memory was allocated for the pointer storage */
Copy link
Contributor

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

@LaszloLango
Copy link
Contributor

LGTM

@LaszloLango LaszloLango added enhancement An improvement style Related to coding style labels Apr 18, 2016
#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);
Copy link
Member

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.

Copy link
Contributor Author

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.

@zherczeg
Copy link
Member

LGTM

…on with 'sizeof'

JerryScript-DCO-1.0-Signed-off-by: Robert Sipka rsipka.uszeged@partner.samsung.com
@robertsipka
Copy link
Contributor Author

rebased with master

@LaszloLango LaszloLango merged commit 9ee4379 into jerryscript-project:master Apr 19, 2016
@robertsipka robertsipka deleted the ecma_value_storage branch November 24, 2016 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement An improvement style Related to coding style

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants