748745 - readString through CDataFinalizer

Status: RESOLVED FIXED

(Core :: js-ctypes, enhancement)

Description

[David Teller [:Yoric] - still alive but not very active]

At the moment, readString may only be called on a CData object. We should be able to call it also on a CDataFinalizer.

Comment 1

[David Teller [:Yoric] - still alive but not very active]

Attachment: Supporting readString in a CDataFinalizer

Comment 2

[Jason Orendorff [:jorendorff]]

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.

Comment 3

[David Teller [:Yoric] - still alive but not very active]

Attachment: Supporting readString in a CDataFinalizer

Fixed, thanks.

Comment 4

[David Teller [:Yoric] - still alive but not very active]

Attachment: Supporting readString in a CDataFinalizer

Oops, turns out that this code could freeze – and that the testsuite somehow does not detect this. Now fixed.

Comment 5

[David Teller [:Yoric] - still alive but not very active]

Attachment: Supporting readString in a CDataFinalizer

Comment 6

[David Teller [:Yoric] - still alive but not very active]

Attachment: Supporting readString in a CDataFinalizer

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.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/14ab85ab1549

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/14ab85ab1549