Last Comment Bug 696450 - Implement pointer increment/decrement in js-ctypes
: Implement pointer increment/decrement in js-ctypes
Status: RESOLVED FIXED
: dev-doc-needed
Product: Core
Classification: Components
Component: js-ctypes (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla11
Assigned To: Josh Matthews [:jdm]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-21 12:01 PDT by Bobby Holley (:bholley) (busy with Stylo)
Modified: 2014-11-07 11:01 PST (History)
5 users (show)
bzbarsky: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Add increment/decrement methods to PointerType jsctype objects. (5.88 KB, patch)
2011-10-21 17:20 PDT, Josh Matthews [:jdm]
bobbyholley: review-
Details | Diff | Splinter Review
Add increment/decrement methods to PointerType jsctype objects. (5.91 KB, patch)
2011-11-08 00:44 PST, Josh Matthews [:jdm]
bobbyholley: review+
Details | Diff | Splinter Review
Add increment/decrement methods to PointerType jsctype objects. (5.91 KB, patch)
2011-11-08 13:08 PST, Josh Matthews [:jdm]
bobbyholley: review+
Details | Diff | Splinter Review

Description Bobby Holley (:bholley) (busy with Stylo) 2011-10-21 12:01:00 PDT
In general we don't really want to support arbitrary pointer arithmetic, because people are likely hurt themselves. If you have var foo of type tType.ptr, and know it corresponds to a buffer of length 5, then you can cast it to tType.array(5).ptr and proceed from there.

However, sometimes you can have buffers of unknown size where you need to iterate and and check for a sentinel value. For these, we should have:

var myFooPtrPlusOne = myFooPtr.increment();

and a corresponding decrement.
Comment 1 Bobby Holley (:bholley) (busy with Stylo) 2011-10-21 14:47:45 PDT
jdm is going to take a crack at this. I'll do the initial review.
Comment 2 Josh Matthews [:jdm] 2011-10-21 17:20:01 PDT
Created attachment 568830 [details] [diff] [review]
Add increment/decrement methods to PointerType jsctype objects.
Comment 3 Bobby Holley (:bholley) (busy with Stylo) 2011-10-21 19:18:41 PDT
Comment on attachment 568830 [details] [diff] [review]
Add increment/decrement methods to PointerType jsctype objects.

Wow, that's one speedy patch! :-)

>+static JSBool
>+IncrementOrDecrement(JSContext* cx, bool increment, jsval* vp)

I think we could do this a little bit better:

1 - Add a PointerType::OffsetBy (or ::Shift, or ::ShiftBy, or any better name you think up), that takes an arbitrary offset and generates a new pointer.

2 - Add it to the namespace PointerType {} block, and add a comment explaining that we're deliberately not adding it to sPointerInstanceFunctions for the aforementioned reasons.

3 - Implement Increment() and Decrement() as simple wrappers that pass 1 and -1 to OffsetBy().


This way, if we ever do decide to support arbitrary pointer arithmetic, it's just a 1-line patch to add it to sPointerInstanceFunctions.


>+
>+  JSObject* pointerType = PointerType::CreateInternal(cx, baseType);
>+  if (!pointerType)
>+    return JS_FALSE;

It's not necessary to construct a new type object here, since the result is of the same type. Just use the one off the 'this' object.
Comment 4 Josh Matthews [:jdm] 2011-11-08 00:44:38 PST
Created attachment 572752 [details] [diff] [review]
Add increment/decrement methods to PointerType jsctype objects.
Comment 5 Bobby Holley (:bholley) (busy with Stylo) 2011-11-08 03:26:32 PST
Comment on attachment 572752 [details] [diff] [review]
Add increment/decrement methods to PointerType jsctype objects.

Looks good. r=bholley. Flagging jorendorff, since he's an actual peer.
Comment 6 Josh Matthews [:jdm] 2011-11-08 12:48:50 PST
Comment on attachment 572752 [details] [diff] [review]
Add increment/decrement methods to PointerType jsctype objects.

Try says this patch is completely broken (a_p.contents is undefined?), so I'm going to investigate.
Comment 7 Josh Matthews [:jdm] 2011-11-08 13:08:25 PST
Created attachment 572976 [details] [diff] [review]
Add increment/decrement methods to PointerType jsctype objects.
Comment 8 Bobby Holley (:bholley) (busy with Stylo) 2011-11-21 10:40:42 PST
Comment on attachment 572976 [details] [diff] [review]
Add increment/decrement methods to PointerType jsctype objects.

dwitte made me a js-ctypes peer, so this can land!
Comment 10 Matt Brubeck (:mbrubeck) 2011-12-07 12:21:05 PST
https://hg.mozilla.org/mozilla-central/rev/c93612dc5922
Comment 11 David Rajchenbach-Teller [:Yoric] (please use "needinfo") 2012-08-06 23:41:41 PDT
FYI, I am starting to need something like OffsetBy in js code.

The main scenario is calling Posix read/write-like functions in which you may need to advance your pointer by an arbitrary number of bytes if the call did not read/write everything at once. Right now, I am casting to an array then casting back, but this is rather awkward, so I have essentially reimplemented OffsetBy in JS code, see bug 771928.

Note You need to log in before you can comment on or make changes to this bug.