Skip to content

Add ArrayBuffer with user specified buffer #2162

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
Merged
merged 1 commit into from
Jan 25, 2018
Merged

Add ArrayBuffer with user specified buffer #2162

merged 1 commit into from
Jan 25, 2018

Conversation

galpeter
Copy link
Contributor

@galpeter galpeter commented Jan 8, 2018

New API functions:

  • jerry_create_arraybuffer_external
  • jerry_arraybuffer_get_pointer

@galpeter galpeter added ES2015 Related to ES2015 features api Related to the public API ecma core Related to core ECMA functionality enhancement An improvement feature request Requested feature labels Jan 8, 2018
@galpeter
Copy link
Contributor Author

galpeter commented Jan 8, 2018

Depends on: #2161

@galpeter galpeter force-pushed the c-api-extarraybuffer branch from ae79e7a to 1acb3be Compare January 8, 2018 17:09
@zherczeg zherczeg mentioned this pull request Jan 11, 2018
Merged
@martijnthe martijnthe mentioned this pull request Jan 11, 2018
Closed
@LaszloLango
Copy link
Contributor

#2161 landed. Please rebase.

@galpeter galpeter force-pushed the c-api-extarraybuffer branch 3 times, most recently from 888edda to 1bfd11b Compare January 16, 2018 09:38
@galpeter
Copy link
Contributor Author

PR rebased

zherczeg
zherczeg requested changes Jan 17, 2018
View reviewed changes
jerry-core/api/jerry.c Outdated
* * Only valid for ArrayBuffers created with jerry_create_arraybuffer_external.
* * This is a high-risk operation as the bounds are not checked
* when accessing the pointer elements.
* * jerry_release_value must be called when the pointer is no longer needed.
Copy link
Member

Choose a reason for hiding this comment

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

release what?

jerry-core/ecma/base/ecma-globals.h Outdated
typedef enum
{
ECMA_ARRAYBUFFER_INTERNAL_MEMORY = 0u, /* ArrayBuffer memory is handled internally. */
ECMA_ARRAYBUFFER_EXTERNAL_MEMORY = (1u << 0), /* Memory provided by the developer. */
Copy link
Member

Choose a reason for hiding this comment

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

Comment: ArrayBuffer created by jerry_create_arraybuffer_external.

jerry-core/ecma/base/ecma-globals.h Outdated
} ecma_arraybuffer_extra_flag_t;

#define ECMA_ARRAYBUFFER_HAS_EXTERNAL_MEMORY(object_p) \
((((ecma_extended_object_t *) object_p)->u.class_prop.extra_info & ECMA_ARRAYBUFFER_EXTERNAL_MEMORY) == 1)
Copy link
Member

Choose a reason for hiding this comment

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

!= 0 is nicer than == 1

jerry-core/ecma/base/ecma-globals.h Outdated
*/
typedef struct
{
void *buffer_p; /**< external buffer pointer */
Copy link
Member

Choose a reason for hiding this comment

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

It is a better technique if the "base" structure is used as the first member. Reason: this ensures proper alignment for structure members.

@galpeter galpeter force-pushed the c-api-extarraybuffer branch from 1bfd11b to b7415b6 Compare January 17, 2018 15:07
@galpeter
Copy link
Contributor Author

@zherczeg I've updated the PR based on the requests.

@galpeter galpeter force-pushed the c-api-extarraybuffer branch from b7415b6 to 95d1a06 Compare January 17, 2018 17:35
LaszloLango
LaszloLango requested changes Jan 22, 2018
View reviewed changes
docs/02.API-REFERENCE.md Outdated

**Summary**

The function allows acces to the contents of the Array Buffer directly.
Copy link
Contributor

Choose a reason for hiding this comment

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

access

docs/02.API-REFERENCE.md Outdated
After using the pointer the [jerry_release_value](#jerry_release_value)
function must be called.

WARNING! This operation is for expert use only! The programmer must
Copy link
Contributor

Choose a reason for hiding this comment

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

Please make the warning text bold: **WARNING!**

docs/02.API-REFERENCE.md Outdated

**Summary**

Create a jerry_value_t representing an ArrayBuffer object with
Copy link
Contributor

Choose a reason for hiding this comment

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

Creates

jerry-core/ecma/operations/ecma-arraybuffer-object.c Outdated
* extend_part
* arraybuffer external info part
*
* @return ecma_object_t *
Copy link
Contributor

Choose a reason for hiding this comment

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

@return Pointer to the created ArrayBuffer object if success, NULL otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

NULL can't be returned here

@galpeter
Add ArrayBuffer with user specified buffer
3b8f218 
New API functions:
 - jerry_create_arraybuffer_external
 - jerry_get_arraybuffer_pointer

JerryScript-DCO-1.0-Signed-off-by: Peter Gal [email protected]
@galpeter galpeter force-pushed the c-api-extarraybuffer branch from 95d1a06 to 3b8f218 Compare January 23, 2018 15:23
@galpeter
Copy link
Contributor Author

@LaszloLango I've updated the PR based on the requests.

zherczeg
zherczeg approved these changes Jan 23, 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 Jan 24, 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

@yichoi yichoi merged commit 4b699e9 into jerryscript-project:master Jan 25, 2018
@LaszloLango LaszloLango mentioned this pull request Feb 5, 2018
Merged
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

@LaszloLango LaszloLango LaszloLango approved these changes

Assignees
No one assigned
Labels
api Related to the public API ecma core Related to core ECMA functionality enhancement An improvement ES2015 Related to ES2015 features feature request Requested feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@galpeter @LaszloLango @zherczeg @yichoi