Merge lp://qastaging/~alan-griffiths/compiz-core/bug.968985 into lp://qastaging/compiz-core

Proposed by Alan Griffiths
Status: Merged
Merged at revision: 3082
Proposed branch: lp://qastaging/~alan-griffiths/compiz-core/bug.968985
Merge into: lp://qastaging/compiz-core
Diff against target: 188 lines (+107/-23)
5 files modified
NEWS (+1/-0)
src/plugin.cpp (+15/-21)
src/plugin/tests/CMakeLists.txt (+19/-0)
src/plugin/tests/test-loader.cpp (+72/-0)
src/plugin/tests/test-plugin.cpp (+0/-2)
To merge this branch: bzr merge lp://qastaging/~alan-griffiths/compiz-core/bug.968985
Reviewer Review Type Date Requested Status
Daniel van Vugt Approve
Sam Spilsbury Approve
Review via email: mp+100108@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2012-03-30.

Description of the change

Fix memory leak in dlloaderListPlugins (lp:968985)

To post a comment you must log in.
Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Crashes on line 268

review: Needs Fixing
Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

==30573== Conditional jump or move depends on uninitialised value(s)
==30573== at 0x4C28293: free (vg_replace_malloc.c:366)
==30573== by 0x4EA2DB1: dlloaderListPlugins(char const*) (plugin.cpp:268)
==30573== by 0x4EA338A: CompPlugin::availablePlugins() (plugin.cpp:545)
==30573== by 0x4E74BDF: compiz::private_screen::PluginManager::mergedPluginList() (screen.cpp:899)
==30573== by 0x4E754F9: compiz::private_screen::PluginManager::updatePlugins() (screen.cpp:959)
==30573== by 0x40358F: CompManager::init() (main.cpp:173)
==30573== by 0x402988: main (main.cpp:234)
==30573==
==30573== Invalid free() / delete / delete[]
==30573== at 0x4C282E0: free (vg_replace_malloc.c:366)
==30573== by 0x4EA2DB1: dlloaderListPlugins(char const*) (plugin.cpp:268)
==30573== by 0x4EA338A: CompPlugin::availablePlugins() (plugin.cpp:545)
==30573== by 0x4E74BDF: compiz::private_screen::PluginManager::mergedPluginList() (screen.cpp:899)
==30573== by 0x4E754F9: compiz::private_screen::PluginManager::updatePlugins() (screen.cpp:959)
==30573== by 0x40358F: CompManager::init() (main.cpp:173)
==30573== by 0x402988: main (main.cpp:234)
==30573== Address 0x1d is not stack'd, malloc'd or (recently) free'd

Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

I still get strange hangs on startup with this branch. Don't know why because it looks right. Another day...

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

Looks fine.

We probably need some kind of guideline as to what tests can and can't do integration wise with the system - could be cases where we're running on a readonly fs.

review: Approve
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Confirmed it works now. I have no idea why this branch was hanging my machine on Friday but it's fine now.

The NEWS update is now misplaced, but I'll fix that by hand.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches