Bug 25928 – [Gtk] Several errors detected by cppcheck

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[xxx]

cppcheck (http://cppcheck.wiki.sourceforge.net/) is greater for checking c++ code!

[./accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:63]: (all) Memory leak: fallbackCache
[./css/CSSParser.cpp:370]: (all) Memory leak: prop
[./css/CSSPrimitiveValue.cpp:79]: (all) Memory leak: colorValueCache
[./page/DOMWindow.cpp:610]: (all) Memory leak: timer
[./platform/graphics/chromium/FontCacheChromiumWin.cpp:231]: (all) Memory leak: fontNameMap
[./platform/graphics/chromium/FontCacheChromiumWin.cpp:294]: (all) Memory leak: fontCmapCache
[./platform/graphics/opentype/OpenTypeUtilities.cpp:375]: (all) Buffer overrun
[./platform/graphics/opentype/OpenTypeUtilities.cpp:384]: (all) Array index out of bounds
[./platform/graphics/opentype/OpenTypeUtilities.cpp:385]: (all) Array index out of bounds
[./platform/graphics/opentype/OpenTypeUtilities.cpp:386]: (all) Array index out of bounds
[./platform/graphics/opentype/OpenTypeUtilities.cpp:387]: (all) Array index out of bounds
[./platform/qt/FileSystemQt.cpp:126]: (all) Memory leak: temp
[./platform/qt/PasteboardQt.cpp:77]: (all) Memory leak: md
[./rendering/style/SVGRenderStyle.cpp:55]: (all) Memory leak: defaultStyle

[Alexey Proskuryakov]

Some of these warnings are false alarms, but some are real.

[Alexey Proskuryakov]

Created attachment 30581 [details]
Fix for cross-platform

This patch fixes all issues affecting cross-platform code (there was only one). Some platform issues look real, too, but I didn't feel qualified to fix them.

> [./css/CSSParser.cpp:370]: (all) Memory leak: prop

Fixed.

> [./css/CSSPrimitiveValue.cpp:79]: (all) Memory leak: colorValueCache
> [./page/DOMWindow.cpp:610]: (all) Memory leak: timer
> [./rendering/style/SVGRenderStyle.cpp:55]: (all) Memory leak: defaultStyle

These are false alarms.

[Eric Seidel (no email)]

Comment on attachment 30581 [details]
Fix for cross-platform

I think the ChangeLog could be slightly clearer.  To state that specifically we would leak if we ever parsed UINT_MAX / sizeof(CSSProperty*) properties (aka ~ 1billion).

Looks great though.

[Alexey Proskuryakov]

Comment on attachment 30581 [details]
Fix for cross-platform

Committed r44044, clearing review flag.

[Alexey Proskuryakov]

FontCacheChromiumWin.cpp ones are also false alarms, so these are ones to be investigated by port maintainers:

[./accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:63]: (all) Memory leak:
fallbackCache
[./platform/graphics/opentype/OpenTypeUtilities.cpp:375]: (all) Buffer overrun
[./platform/graphics/opentype/OpenTypeUtilities.cpp:384]: (all) Array index out
of bounds
[./platform/graphics/opentype/OpenTypeUtilities.cpp:385]: (all) Array index out
of bounds
[./platform/graphics/opentype/OpenTypeUtilities.cpp:386]: (all) Array index out
of bounds
[./platform/graphics/opentype/OpenTypeUtilities.cpp:387]: (all) Array index out
of bounds
[./platform/qt/FileSystemQt.cpp:126]: (all) Memory leak: temp
[./platform/qt/PasteboardQt.cpp:77]: (all) Memory leak: md

[Gustavo Noronha (kov)]

(In reply to comment #0)
> [./accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:63]: (all) Memory leak:
> fallbackCache

My reading of the code indicates this is a false-positive. fallbackCache is meant to be alive from the first call onwards (it's static).

[Andreas Kling]

(In reply to comment #5)
> [./platform/qt/PasteboardQt.cpp:77]: (all) Memory leak: md

False positive - ownership is transferred in QClipboard::setMimeData()

[Andreas Kling]

Actually, this is also a false positive:
[./platform/qt/FileSystemQt.cpp:126]: (all) Memory leak: temp

Removing Qt keyword.

[Gustavo Noronha (kov)]

I think we can close this bug, then, since the only GTK+ one was also a false-positive. Thanks!