Closed
Bug 239499
Opened 21 years ago
Closed 16 years ago
64bit safe code for mozilla/content (WinXP AMD64) part 2
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: m_kato, Assigned: sicking)
References
Details
(Keywords: 64bit)
Attachments
(1 file, 3 obsolete files)
1.86 KB,
patch
|
peterv
:
review+
peterv
:
superreview+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7a) Gecko/20040219
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7a) Gecko/20040219
sAttrValue and nsAttrAndChildArray uses unsigned long as pointer. We need to
change it to PRUword for porting of WinXP 64bit. About it, please see bug 229722.
Reproducible: Always
Steps to Reproduce:
not need. This is porting issue
Actual Results:
not need. This is porting issue
Expected Results:
not need. This is porting issue
Reporter | ||
Comment 1•21 years ago
|
||
Comment 3•21 years ago
|
||
Comment on attachment 145370 [details] [diff] [review]
patch of mozilla/content at 2004/04/03
Requesting r= from pkw since he has a 64bit platform around to test.
Attachment #145370 -
Flags: review?(pkw)
![]() |
||
Comment 4•21 years ago
|
||
Um... ccing module owners and such so they have a clue as to what's going on in
their module.
Assignee | ||
Comment 5•21 years ago
|
||
Comment on attachment 145370 [details] [diff] [review]
patch of mozilla/content at 2004/04/03
Please remove the typedefs entierly and use the PRUword type directly instead.
Attachment #145370 -
Flags: review?(pkw) → review-
Reporter | ||
Comment 6•21 years ago
|
||
Attachment #145370 -
Attachment is obsolete: true
Reporter | ||
Updated•21 years ago
|
Attachment #169010 -
Flags: review?
Reporter | ||
Updated•21 years ago
|
Assignee: general → m_kato
Reporter | ||
Updated•21 years ago
|
Status: NEW → ASSIGNED
Attachment #169010 -
Flags: review? → review?(bugmail)
Assignee | ||
Comment 7•19 years ago
|
||
Can you not use PRWord?
Assignee | ||
Comment 8•19 years ago
|
||
This should be a more stable approach. There is clearly a need for this type across our codebase, so lets create a proper define in XPCOM that everyone can use.
We should consider removing this type across the tree once NSPR has a proper pointer-sized integer type. However even then it might make sense to have a type that is specifically for use in bitfields (for one so that people don't forget to make it unsigned).
Assignee: m_kato → bugmail
Attachment #169010 -
Attachment is obsolete: true
Attachment #217225 -
Flags: superreview?(darin)
Attachment #217225 -
Flags: review?(darin)
Attachment #169010 -
Flags: review?(bugmail)
Comment 9•19 years ago
|
||
Comment on attachment 217225 [details] [diff] [review]
centralize PtrBits
>Index: xpcom/base/nscore.h
>+/**
>+ * Generic type for a bitfield that is the size of a void*
>+ *
>+ * NOTE! This define will change or go away once NSPR provides a
>+ * real datatype for integers whos size is the size of a void*,
>+ * but it is the recommended type until then.
>+ */
>+typedef PRUword PtrBits;
>+
Couple comments:
1) strange for this type to not begin with "ns" or "PR".. nscore.h is included by anyone trying to use XPCOM so we have to be sensitive about conflicting with user-defined types. namespacing our types, defines, and symbols probably matters a lot.
2) nspr seems to define the type.. PRUword, no? at least, this code seems inconsistent with the comment.
In short, I'm not sure that we want exactly this added to nscore.h :(
Assignee | ||
Comment 10•19 years ago
|
||
(In reply to comment #9)
> 2) nspr seems to define the type.. PRUword, no? at least, this code seems
> inconsistent with the comment.
For two reasons:
1. PR(U)word have a big comment at the top of it saying not to use this type. Why i'm not really sure and atm there is no better alternative. See bug 313297, bug 229722.
2. Even with PR(U)word around people seem to have felt it neccesary to create a new define, as proven by the fact that most of the defines point to PRUword.
So I do think we want a special define, it's just a matter of where it should live and what it should be named. And I'd prefer not to wait for nspr.
> 1) strange for this type to not begin with "ns" or "PR".. nscore.h is included
> by anyone trying to use XPCOM so we have to be sensitive about conflicting
> with user-defined types. namespacing our types, defines, and symbols probably
> matters a lot.
Good point.. half of the idea with this apprach was to not have to go through the tree and mess up the blame twice, once now and once if/when NSPR get their act together.
But it's not so bad so maybe i should just rename it nsptrbits?
Comment 11•19 years ago
|
||
Let's just fix bug 313297. Propose a patch in that bug :-)
Comment 12•19 years ago
|
||
Comment on attachment 217225 [details] [diff] [review]
centralize PtrBits
ok, minusing this patch in favor of nspr solution. thanks!
Attachment #217225 -
Flags: superreview?(darin)
Attachment #217225 -
Flags: superreview-
Attachment #217225 -
Flags: review?(darin)
Attachment #217225 -
Flags: review-
Comment 13•19 years ago
|
||
Does this mean we should add bug 313297 as a dependency of bug 237202 as well then?
Reporter | ||
Updated•17 years ago
|
Blocks: tracking_win64
Reporter | ||
Comment 15•16 years ago
|
||
same as bug 478190. This part is for mozilla/content
Attachment #217225 -
Attachment is obsolete: true
Attachment #370577 -
Flags: review?(peterv)
Updated•16 years ago
|
Attachment #370577 -
Flags: superreview+
Attachment #370577 -
Flags: review?(peterv)
Attachment #370577 -
Flags: review+
Reporter | ||
Comment 16•16 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•