Closed
Bug 732936
Opened 9 years ago
Closed 9 years ago
Feeding ArrayBuffer to js-ctypes
Categories
(Core :: js-ctypes, enhancement)
Core
js-ctypes
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: Yoric, Assigned: Yoric)
References
Details
(Keywords: dev-doc-needed)
Attachments
(1 file, 3 obsolete files)
7.90 KB,
patch
|
Details | Diff | Splinter Review |
It would be nice to be able to use a TypedArray as a C array/pointer when performing a js-ctypes call. I believe it would be rather simple to adapt ImplicitConvert for that purpose.
Assignee | ||
Updated•9 years ago
|
Severity: normal → enhancement
OS: Mac OS X → All
Hardware: x86 → All
Assignee | ||
Comment 1•9 years ago
|
||
Attachment #621509 -
Flags: feedback?(jorendorff)
Comment 2•9 years ago
|
||
Comment on attachment 621509 [details] [diff] [review] First prototype Hmm. I like everything here except that nothing is holding on to the TypedArray, so it would be easy for this pointer to be taken, implicitly, and then for the buffer to be GC'd, leaving a dangling ctypes pointer. One way around that is to move this code to ExplicitConvert. Another is to create utility functions: ctypes.bufferPtr(buffer or typedarray) ctypes.addressOfElement(typedarray, index) Another is to create a property on the pointer CData object that refers back to the TypedArray (but that's lame). Another possibility is to allow this conversion only if isArgument is true, similar to what we do for strings. For arguments, the conversion is safe at least for the duration of the C function call, since all arguments are rooted on the JS stack. Or we can leave it like it is and be really careful. It's up to you, you're the main ctypes user right now.
Attachment #621509 -
Flags: feedback?(jorendorff)
Comment 3•9 years ago
|
||
Actually, this is just the same thing we already do for js-ctypes arrays. It should be totally fine.
Comment 4•9 years ago
|
||
Comment on attachment 621509 [details] [diff] [review] First prototype no changes needed, post an updated patch for review One note, there are a few bits in here that seem to belong to a different patch: the change to dom/system/OSFileConstants.cpp, and the one to toolkit/components/osfile/osfile_unix_front.jsm.
Attachment #621509 -
Flags: feedback+
Assignee | ||
Comment 5•9 years ago
|
||
Same patch, with a few more comments and without parasite code.
Assignee: nobody → dteller
Attachment #621509 -
Attachment is obsolete: true
Attachment #624009 -
Flags: review?(jorendorff)
Comment 6•9 years ago
|
||
Comment on attachment 624009 [details] [diff] [review] mplicitConvert ArrayBuffer to array or pointer Review of attachment 624009 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/ctypes/tests/unit/test_jsctypes.js.in @@ +61,1 @@ > } Did you want these new print calls checked in? They don't hurt anything, afaik, so it's fine with me. @@ +1910,5 @@ > } > } > > function run_ArrayType_tests() { > + do_print("run_ArrayType_tests"); Here's another do_print(), just in case you meant to remove them. @@ +2051,5 @@ > + c.value = c_arraybuffer; > + } catch(x) { > + exn = x; > + } > + do_check_true(!!exn); Use do_check_throws.
Attachment #624009 -
Flags: review?(jorendorff) → review+
Assignee | ||
Comment 7•9 years ago
|
||
Attachment #624009 -
Attachment is obsolete: true
Assignee | ||
Comment 8•9 years ago
|
||
Attachment #625748 -
Attachment is obsolete: true
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 10•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9320ebf5821d
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 12•9 years ago
|
||
Title is actually a misnomer.
Summary: Feeding TypedArray to js-ctypes → Feeding ArrayBuffer to js-ctypes
Updated•9 years ago
|
Keywords: dev-doc-needed
Comment 13•6 years ago
|
||
Documentation started here: https://developer.mozilla.org/en-US/docs/Mozilla/js-ctypes/Using_js-ctypes/Working_with_ArrayBuffers Probalby needs some serious technical review.
You need to log in
before you can comment on or make changes to this bug.
Description
•