Closed
Bug 748745
Opened 12 years ago
Closed 12 years ago
readString through CDataFinalizer
Categories
(Core :: js-ctypes, enhancement)
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: Yoric, Assigned: Yoric)
References
Details
Attachments
(1 file, 4 obsolete files)
5.99 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
At the moment, readString may only be called on a CData object. We should be able to call it also on a CDataFinalizer.
Assignee | ||
Comment 1•12 years ago
|
||
Assignee: nobody → dteller
Status: NEW → UNCONFIRMED
Ever confirmed: false
Attachment #618266 -
Flags: review?(jorendorff)
Comment 2•12 years ago
|
||
Comment on attachment 618266 [details] [diff] [review] Supporting readString in a CDataFinalizer I don't see where test_finalizer_rel_string_t frees the memory allocated for the string in test_finalizer_acq_string_t. It's a leak, right? > acquire_void_ptr = library.declare("test_finalizer_acq_ptr_t", > ctypes.default_abi, >- ctypes.int32_t.ptr, >+ ctypes.void_t.ptr, > ctypes.size_t); > dispose_void_ptr = library.declare("test_finalizer_rel_ptr_t", > ctypes.default_abi, > ctypes.void_t, > ctypes.int32_t.ptr); While you're fixing acquire_void_ptr, I think the last line here (for dispose_void_ptr) should get the same change. r=me with those changes.
Attachment #618266 -
Flags: review?(jorendorff) → review+
Assignee | ||
Comment 4•12 years ago
|
||
Oops, turns out that this code could freeze – and that the testsuite somehow does not detect this. Now fixed.
Attachment #620715 -
Attachment is obsolete: true
Assignee | ||
Comment 5•12 years ago
|
||
Attachment #621510 -
Attachment is obsolete: true
Assignee | ||
Comment 6•12 years ago
|
||
Turns out that the error was caused by the interaction with the patch for bug 723530 – and that while it broke the xpcshell tests, it did so silently. Here is a patch that should solve the issue.
Attachment #621511 -
Attachment is obsolete: true
Attachment #621515 -
Flags: review?(jorendorff)
Assignee | ||
Updated•12 years ago
|
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Updated•12 years ago
|
Attachment #621515 -
Flags: review?(jorendorff) → review+
Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
Comment 8•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/14ab85ab1549
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•