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)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: m_kato, Assigned: sicking)

References

Details

(Keywords: 64bit)

Attachments

(1 file, 3 obsolete files)

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
Blocks: 237202
Nice catch...
Status: UNCONFIRMED → NEW
Ever confirmed: true
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)
Keywords: 64bit
Um... ccing module owners and such so they have a clue as to what's going on in their module.
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-
Attached patch patch for mozilla/content (obsolete) — Splinter Review
Attachment #145370 - Attachment is obsolete: true
Attachment #169010 - Flags: review?
Assignee: general → m_kato
Status: NEW → ASSIGNED
Attachment #169010 - Flags: review? → review?(bugmail)
Can you not use PRWord?
Attached patch centralize PtrBits (obsolete) — Splinter Review
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 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 :(
(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?
Let's just fix bug 313297. Propose a patch in that bug :-)
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-
Does this mean we should add bug 313297 as a dependency of bug 237202 as well then?
Yes, we should do it.
Blocks: 313297
No longer blocks: 313297
Attached patch patch v3Splinter Review
same as bug 478190. This part is for mozilla/content
Attachment #217225 - Attachment is obsolete: true
Attachment #370577 - Flags: review?(peterv)
Attachment #370577 - Flags: superreview+
Attachment #370577 - Flags: review?(peterv)
Attachment #370577 - Flags: review+
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: