Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix ecma_op_object_put return value not being handled #2514

Merged
merged 1 commit into from
Sep 6, 2018
Merged

Fix ecma_op_object_put return value not being handled #2514

merged 1 commit into from
Sep 6, 2018

Conversation

DanielBallaSZTE
Copy link

Fixes #2487
Fixes #2488

Co-authored-by: Robert Fancsik [email protected]
JerryScript-DCO-1.0-Signed-off-by: Daniel Balla [email protected]

Fix ecma_op_object_put return value not being handled
b6e784a 
Fixes jerryscript-project#2487
Fixes jerryscript-project#2488

Co-authored-by: Robert Fancsik [email protected]
JerryScript-DCO-1.0-Signed-off-by: Daniel Balla [email protected]
zherczeg
zherczeg approved these changes Sep 6, 2018
View reviewed changes
Copy link
Member

@zherczeg zherczeg left a comment

Choose a reason for hiding this comment

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

LGTM

LaszloLango
LaszloLango approved these changes Sep 6, 2018
View reviewed changes
Copy link
Contributor

@LaszloLango LaszloLango left a comment

Choose a reason for hiding this comment

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

LGTM

PS.: Almost every call of the ecma_op_object_put function miss the check of the return value which is potential bug in our code base. I think we should raise an issue for that and investigate why are those checks missing.

akosthekiss
akosthekiss approved these changes Sep 6, 2018
View reviewed changes
Copy link
Member

@akosthekiss akosthekiss left a comment

Choose a reason for hiding this comment

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

LGTM

@akosthekiss akosthekiss merged commit 8f64339 into jerryscript-project:master Sep 6, 2018
@DanielBallaSZTE DanielBallaSZTE mentioned this pull request Nov 26, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zherczeg zherczeg zherczeg approved these changes

@akosthekiss akosthekiss akosthekiss approved these changes

@LaszloLango LaszloLango LaszloLango approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@DanielBallaSZTE @zherczeg @akosthekiss @LaszloLango