Closed Bug 590052 Opened 14 years ago Closed 14 years ago

eliminate usage of base serialization functions for values without strict sizes

Categories

(Core :: IPC, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- beta7+

People

(Reporter: jaas, Assigned: jaas)

References

Details

Attachments

(1 file, 4 obsolete files)

We should eliminate usage of the following functions from our ipc code because they deal with values that don't have the same size on all architectures. [Read/Write]Long [Read/Write]ULong [Read/Write]Size [Read/Write]IntPtr We should also move "int" and "short" versions of these functions to more explicitly-sized versions but they happen to be the same size on i386 and x86_64 so they are less of a priority.
OS: Mac OS X → All
Hardware: x86 → All
Blocks: 559142
The easiest way to do this is presumably to remove the functions entirely, right?
We could temporarily hijack these functions to read/write the i386 size types regardless of the architecture with a warning/abort on truncation. This would let us progress around this problem while the references are being removed and would also find the places where the i386 types are too small. That said I agree that the desired long term fix is to remove these functions entirely.
Attached patch fix v1.0 (always serialize as 64-bit value) (obsolete) (deleted) — Splinter Review
I had this patch around but I hadn't tested it yet when I filed this bug. It is somewhat more difficult to just kill the functions because they end up being used implicitly by typedef'd param types so I coded it up such that variable sized types are always serialized as the larger type. This way a 64-bit -> 64-bit message could use the whole range of the type, but the downside is that if a value requiring the full 64 type is sent from a 64-bit arch to a 32-bit arch it'll get incorrectly truncated. I don't think this will be a problem in practice.
Attached patch fix v1.1 (obsolete) (deleted) — Splinter Review
Attachment #468763 - Attachment is obsolete: true
Attachment #468773 - Flags: review?(jones.chris.g)
> if a value requiring the full 64 type is sent from a 64-bit arch to a 32-bit > arch it'll get incorrectly truncated. I don't think this will be a problem in > practice. It should be easy to detect truncation and throw a warning (or abort). This would make errors more obvious and make it easy to trace what messages require the larger 64-bit types.
Yeah, we should add debug assertions. I'll do that in the next patch revision.
Comment on attachment 468773 [details] [diff] [review] fix v1.1 r=me with overflow guard asserts. Now we get to see which platforms give us |long| handles > 1<<31 :/.
Attachment #468773 - Flags: review?(jones.chris.g) → review+
blocking2.0: --- → beta6+
Attached patch fix v1.2 (obsolete) (deleted) — Splinter Review
Attachment #468773 - Attachment is obsolete: true
Attached patch fix v1.3 (obsolete) (deleted) — Splinter Review
Attachment #469582 - Attachment is obsolete: true
Attached patch fix v1.4 (deleted) — Splinter Review
Attachment #469645 - Attachment is obsolete: true
Comment on attachment 469691 [details] [diff] [review] fix v1.4 There are enough changes here that I'd like to get this reviewed again.
Attachment #469691 - Flags: review?(jones.chris.g)
Comment on attachment 469691 [details] [diff] [review] fix v1.4 Nice.
Attachment #469691 - Flags: review?(jones.chris.g) → review+
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: