170160 - s/sprintf/AppendInt/ in layout/xul

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Daniel Bratell]

This is a part of bug 92064.

In several places we put up a big char buffer on the stack, call sprintf to fill
that buffer with a number as a string and finally convert that buffer to unicode. 

Faster and smarter is to use AppendInt on a nsAutoString. Patch coming up.

Comment 1

[Daniel Bratell]

Attachment: Use AppendInt instead of sprintf

Here's a patch. Who should I call for a review? Hyatt? The changes are trivial.

Comment 2

[Brendan Eich [:brendan]]

Try cc'ing some people who could r= and sr= in addition to hyatt.  Heck, I'll
sr= the patch right now.

/be

Comment 3

[Brendan Eich [:brendan]]

Comment on attachment 100139 [details] [diff] [review]
Use AppendInt instead of sprintf

Nit: why not append Str and not mess with interCaps when naming the
nsAutoString variable: newpos => newposStr, not newPosStr.

sr=[e-mail] with a valid r=.

/be

Comment 4

[Daniel Bratell]

I'll make that change before checking in if I get a r=.

Comment 5

[Daniel Bratell]

Comment on attachment 100139 [details] [diff] [review]
Use AppendInt instead of sprintf

Trying another reviewer...

Comment 6

[Daniel Bratell]

Taking the bug

Comment 7

[Daniel Bratell]

Checked in patch.

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

+     printf("Current Pos=%s\n",newPosStr.get());
this won't work now, because newPosStr is an nsAutoString, so you'd get back a
PRUnichar*...

Comment 9

[Daniel Bratell]

That should be fixed in the code I checked in. That said, since it's ifdefed
away,  it hasn't been put through my compiler.

Comment 10

[Christian :Biesinger (don't email me, ping me on IRC)]

ah, indeed. I had only looked at the patch on this bug, not at what was actually
checked in.