Skip to content

Introducing getters and setters for compressed pointers in 'mem' component #57

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

Conversation

ruben-ayrapetyan
Copy link
Contributor

Adding MEM_CP_{GET_[NON_NULL_]POINTER, SET_[NON_NULL_]POINTER} interfaces similar to ECMA_{GET, SET}[NON_NULL]POINTER in ecma, that can be used throughout the engine.

@ruben-ayrapetyan ruben-ayrapetyan added this to the Core ECMA features milestone May 13, 2015
@ruben-ayrapetyan ruben-ayrapetyan added normal memory management Related to memory management or garbage collection development Feature implementation labels May 13, 2015
@egavrin
Copy link
Contributor

egavrin commented May 13, 2015

OK to commit

@ruben-ayrapetyan ruben-ayrapetyan merged commit 5efa522 into master May 13, 2015
@ruben-ayrapetyan ruben-ayrapetyan deleted the Ruben-introduce-mem-compressed-pointers-getters-setters branch May 13, 2015 18:17
@ruben-ayrapetyan ruben-ayrapetyan restored the Ruben-introduce-mem-compressed-pointers-getters-setters branch May 13, 2015 19:11
non_compressed_pointer = __temp_pointer; \
} while (0); \
\
if (unlikely ((non_compressed_pointer) == NULL)) \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider the following example usage:

 if (something_should_be_true(..))
    MEM_CP_SET_POINTER(...);
else
   ....

that example would lead to a compile error, and if you leave out the else case then it will compile, but won't do what you really wanted.

The macro should have been written in the following format, to avoid such problems:

#define MEM_CP_SET_POINTER(cp_value, non_compressed_pointer) \
do \
{ \
  auto ....  \
  ....  \
  if ... \
  else ... \
} while (0)

Note that there is no semicolon at the end of the while (0).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, Jerryscript's style requires braces on every control construction.

So, the if would be written as the following:

 if (something_should_be_true(..))
 {
    MEM_CP_SET_POINTER(...);
 }
 else
 {
   ....
 }

And, the example would expand to:

 if (something_should_be_true(..))
 {
   do
   {
     auto __temp_pointer = non_compressed_pointer;
     non_compressed_pointer = __temp_pointer;
   } while (0);

   if (unlikely ((non_compressed_pointer) == NULL))
   {
     (cp_value) = MEM_CP_NULL;
   }
   else
   {
     MEM_CP_SET_NON_NULL_POINTER (cp_value, non_compressed_pointer);
   }
 }
 else
 {
   ....
 }

Anyway, thanks for the advice.
I agree that your implementation of the macro is more secure and should have been used in the case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Feature implementation memory management Related to memory management or garbage collection normal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants