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)
Core
IPC
Tracking
()
RESOLVED
FIXED
| Tracking | Status | |
|---|---|---|
| blocking2.0 | --- | beta7+ |
People
(Reporter: jaas, Assigned: jaas)
References
Details
Attachments
(1 file, 4 obsolete files)
|
7.74 KB,
patch
|
cjones
:
review+
|
Details | Diff | Splinter Review |
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.
|
||
Comment 1•14 years ago
|
||
The easiest way to do this is presumably to remove the functions entirely, right?
|
||
Comment 2•14 years ago
|
||
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.
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.
Attachment #468763 -
Attachment is obsolete: true
Attachment #468773 -
Flags: review?(jones.chris.g)
|
|
||
Comment 5•14 years ago
|
||
> 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+
Attachment #468773 -
Attachment is obsolete: true
Attachment #469582 -
Attachment is obsolete: true
|
Assignee | |
Comment 10•14 years ago
|
||
Attachment #469645 -
Attachment is obsolete: true
|
|
Assignee | |
Comment 11•14 years ago
|
||
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+
|
|
Assignee | |
Comment 13•14 years ago
|
||
pushed to mozilla-central http://hg.mozilla.org/mozilla-central/rev/da862772518d
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.
Description
•