131417 - Memory leak in SetUpFontCharSetInfo()

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Peter A Jonsson]

In nsFontMetricsGTK.cpp:1992 the following line occurs:
 NS_WARNING(nsPrintfCString("\n\ncharset = %s", atomToName(charset)).get());

This leaks a char * which atomToName() allocates. I'll attach a patch below.

Comment 1

[Peter A Jonsson]

Attachment: free the memory allocated by atomToName()

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 74515 [details] [diff] [review]
free the memory allocated by atomToName()

r=bzbarsky.  Go ahead and ask for sr.  :)

Comment 3

[Stuart Parmenter]

r=pavlov.  get blizzard or tor to sr=

Comment 4

[Christopher Blizzard (:blizzard)]

Comment on attachment 74515 [details] [diff] [review]
free the memory allocated by atomToName()

sr=blizzard

Comment 5

[Asa Dotzler [:asa]]

Comment on attachment 74515 [details] [diff] [review]
free the memory allocated by atomToName()

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 6

[Axel Hecht]

as NS_WARNING goes to /* nothing */ for non-debug builds, shouldn't this
code be 
#ifdef DEBUG
?

Comment 7

[jag (Peter Annema)]

Because of the semi-colon?

Comment 8

[Peter A Jonsson]

This patch adds a few more calls compared to how it was, even in an optimized
build. The NS_WARNING() is not a part of an optimized build - with this patch
there will be a few more syscalls during startup. Since startup time is
important it's probably a good idea to avoid adding more calls that will consume
CPU cycles (and the result isn't used unless it's a debug build).

Comment 9

[jag (Peter Annema)]

Doh, yes, I see what you mean now (wasn't quite awake before). All that new code
should be in a #ifdef DEBUG.

Comment 10

[Peter A Jonsson]

Attachment: Patch, with #ifdef DEBUG.

New patch with the ifdefs added. Does the r/sr/a carry over since it is exactly
the same code with only a new ifdef?

Comment 11

[jag (Peter Annema)]

Comment on attachment 75558 [details] [diff] [review]
Patch, with #ifdef DEBUG.

Depends on the patch, but sr=jag on the added ifdef, and I'm copying over the
r= and a=.

Comment 12

[Boris Zbarsky [:bzbarsky]]

checked in on the trunk

Comment 13

[Roland Mainz]

Reopen - you forgot to patch the Xlib version of the code... ;-(

Comment 14

[Roland Mainz]

Taking...

Comment 15

[Roland Mainz]

Attachment: Xlib port of the GTK+ patch

Comment 16

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 75685 [details] [diff] [review]
Xlib port of the GTK+ patch

r=bzbarsky

Comment 17

[Scott Collins]

Comment on attachment 75685 [details] [diff] [review]
Xlib port of the GTK+ patch

sr=scc

Comment 18

[Asa Dotzler [:asa]]

Comment on attachment 75685 [details] [diff] [review]
Xlib port of the GTK+ patch

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 19

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

checked in on trunk