Skip to content

Update the API changes in mbed targets #1175

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

Merged
merged 1 commit into from
Aug 11, 2016

Conversation

polaroi8d
Copy link
Contributor

Updating the ports because the API of JerryScript has been changed recently.

@LaszloLango LaszloLango added bug Undesired behaviour jerry-port Related to the port API or the default port implementation labels Jun 30, 2016
@@ -54,7 +54,7 @@ int js_entry (const char *source_p, const size_t source_size)
}
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

seems a trailing white space... is this intended?

@LaszloLango
Copy link
Contributor

#1177 is merged. docs/02.API-EXAMPLES.md is up to date, but docs/02.API-REFERENCE.md is not. Let me know, if you need any help with the update.

@LaszloLango
Copy link
Contributor

@polaroi8d, please check #1201 too.

@polaroi8d
Copy link
Contributor Author

Thanks @LaszloLango, I will check it.

@@ -31,11 +31,10 @@

#define DECLARE_HANDLER(NAME) \
static bool \
Copy link
Contributor

Choose a reason for hiding this comment

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

static jerry_value_t \

@polaroi8d
Copy link
Contributor Author

Tested on FRDM-K64F, works fine.


return 0;
return ret_val;
Copy link
Contributor

Choose a reason for hiding this comment

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

why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mixed with the DECLARE_HANDLER return value.

@zherczeg
Copy link
Member

Only style issues remained.

@LaszloLango
Copy link
Contributor

LGTM

port = (int)JS_VALUE_TO_NUMBER (&args_p[0]);
value = (int)JS_VALUE_TO_NUMBER (&args_p[1]);
port = (int) jerry_get_number_value (args[0]);
value = (int) jerry_get_number_value (args[1]);
Copy link
Member

Choose a reason for hiding this comment

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

Is it ensured that the arguments are numbers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, thanks for the hint.

if (args_cnt < 2)
{
return false;
ret_val = jerry_create_boolean (false);
printf ("Error: invalid arguments number!\r\n");
Copy link
Member

Choose a reason for hiding this comment

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

You should add a return after this.

@polaroi8d
Copy link
Contributor Author

Updated the PR, added returns if the execution false.

return false;
jerry_release_value (global_object_val);
jerry_release_value (reg_function);
jerry_release_value (set_result);
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this a compiler error? set_result has not initialized yet.

Copy link
Member

Choose a reason for hiding this comment

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

This is bad news if the compiler does not warn about these :(

@polaroi8d polaroi8d force-pushed the api_changes branch 2 times, most recently from c59ab2b to 69b4f51 Compare August 8, 2016 11:26
jerry_api_release_object (global_obj_p);
jerry_value_t set_result = jerry_set_property (global_object_val,
jerry_name,
reg_function);
Copy link
Member

Choose a reason for hiding this comment

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

Indentation error.

@zherczeg
Copy link
Member

zherczeg commented Aug 8, 2016

LGTM after the style fix.

@polaroi8d polaroi8d force-pushed the api_changes branch 3 times, most recently from 0e5a2e2 to 155ec47 Compare August 11, 2016 12:21
- Update the API changes in mbed targets
- Build fix for mbed target after the build system patch.

JerryScript-DCO-1.0-Signed-off-by: Levente Orban [email protected]
@bzsolt bzsolt merged commit 91e2912 into jerryscript-project:master Aug 11, 2016
@polaroi8d polaroi8d deleted the api_changes branch January 6, 2017 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Undesired behaviour jerry-port Related to the port API or the default port implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants