696450 - Implement pointer increment/decrement in js-ctypes

Status: RESOLVED FIXED

(Core :: js-ctypes, defect)

Description

[Bobby Holley (:bholley)]

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)]

jdm is going to take a crack at this. I'll do the initial review.

Comment 2

[Josh Matthews [:jdm]]

Attachment: Add increment/decrement methods to PointerType jsctype objects.

Comment 3

[Bobby Holley (:bholley)]

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]]

Attachment: Add increment/decrement methods to PointerType jsctype objects.

Comment 5

[Bobby Holley (:bholley)]

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]]

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]]

Attachment: Add increment/decrement methods to PointerType jsctype objects.

Comment 8

[Bobby Holley (:bholley)]

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 9

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c93612dc5922

Comment 10

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/c93612dc5922

Comment 11

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

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.