AIX debug build fails after recent AppendInt changes

RESOLVED FIXED

Status

()

Core
String
RESOLVED FIXED
14 years ago
14 years ago

People

(Reporter: Philip K. Warren, Assigned: Philip K. Warren)

Tracking

({fixed1.7})

Trunk
Other
AIX
fixed1.7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Assignee)

Description

14 years ago
AIX debug builds fail to build after the recent changes to AppendInt. The error
occurs when compiling nsContentTestNode.cpp. I will attach a build log from a
recent build.
(Assignee)

Comment 1

14 years ago
Created attachment 147739 [details]
Failing build log
(Assignee)

Comment 2

14 years ago
Created attachment 147743 [details] [diff] [review]
Patch v1
Assignee: string → pkw
Status: NEW → ASSIGNED
(Assignee)

Comment 3

14 years ago
Comment on attachment 147743 [details] [diff] [review]
Patch v1

I used the initial suggestion from Brendan in
http://bugzilla.mozilla.org/show_bug.cgi?id=133801#c6.
Attachment #147743 - Flags: superreview?(darin)
Attachment #147743 - Flags: review?(darin)

Comment 4

14 years ago
I guess this is caused by the fact that we now have an AppendInt that takes a
PRUint32 as well as the one that takes a PRInt32, and NS_PTR_TO_INT32 doesn't
include a cast to specify the INT32 type.

Comment 5

14 years ago
Comment on attachment 147743 [details] [diff] [review]
Patch v1

sr=darin

requesting MOA=/r= from jst.
Attachment #147743 - Flags: superreview?(darin)
Attachment #147743 - Flags: superreview+
Attachment #147743 - Flags: review?(jst)
Attachment #147743 - Flags: review?(darin)
Comment on attachment 147743 [details] [diff] [review]
Patch v1

     aResult.Append(PRUnichar('@'));
-    aResult.AppendInt(NS_PTR_TO_INT32(aContent), 16);
+    AppendASCIItoUTF16(nsPrintfCString("%p", aContent), aResult);

Wanna remove the first Append call, and make the format string to
nsPrintfCString be "@%p"?

r=jst
Attachment #147743 - Flags: review?(jst) → review+

Comment 7

14 years ago
hmm... i was just reminded of the fact that nsPrintfCString uses a 16 byte
buffer by default.  it uses PR_vsnprintf to write to that buffer, which means
that a 64-bit address in hex will not entirely fit in the buffer.  with the '@'
character added, that means the address will be truncated to 14 hex characters.
 you can also use the other form of the nsPrintfCString constructor:

  nsPrintfCString(18, "@%p", aContent)

this causes an 18 character, malloc'd buffer to be used instead.
(Assignee)

Comment 8

14 years ago
Created attachment 147751 [details] [diff] [review]
Patch v2

Patch that was checked in.

Comment 9

14 years ago
Comment on attachment 147751 [details] [diff] [review]
Patch v2

sr=darin
Attachment #147751 - Flags: superreview+
(Assignee)

Comment 10

14 years ago
Fix checked in.

Checking in nsContentTestNode.cpp;
/cvsroot/mozilla/content/xul/templates/src/nsContentTestNode.cpp,v  <-- 
nsContentTestNode.cpp
new revision: 1.16; previous revision: 1.15
done
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
(Assignee)

Comment 11

14 years ago
Comment on attachment 147751 [details] [diff] [review]
Patch v2

Requesting approval for a simple build break fix, which only affects debug
builds.
Attachment #147751 - Flags: approval1.7?

Comment 12

14 years ago
Comment on attachment 147751 [details] [diff] [review]
Patch v2

a=mkaply
Attachment #147751 - Flags: approval1.7? → approval1.7+
(Assignee)

Comment 13

14 years ago
Checked into 1.7 branch.

Checking in nsContentTestNode.cpp;
/cvsroot/mozilla/content/xul/templates/src/nsContentTestNode.cpp,v  <-- 
nsContentTestNode.cpp
new revision: 1.14.14.1; previous revision: 1.14
done
Keywords: fixed1.7
You need to log in before you can comment on or make changes to this bug.