[vlc-devel] [Re:] [vlc-commits] commit: Qt: delete unneeded MenuItemData (Jakob Leben )

[Jakob Leben]

On Fri, Apr 2, 2010 at 10:52 PM, brezhoneg1 <brezhoneg1 at yahoo.fr> wrote:

> Hi,
>
> >Qt: delete unneeded MenuItemData (Jakob Leben
> eacddb63e8a5bda42395ee79b5b27db6408] | committer: Jakob Leben
> >
> >Qt: delete unneeded MenuItemData
> >
>
> This patch was singled out as the one causing a regression for skins2 (a
> user of qt4 menus),
>  namely skins2 yet again crashing at exit on both win32 and Linux.
>
>
I can not reproduce this on Linux (Kubuntu Karmic with Qt 4.6), neither in
skins2 nor qt interface, neither by exiting via top-right close button nor
via right-click menu. I also don't see anything directly relevant in
Valgrind's reports - only some reports in relation exclusively to inside of
Qt. Could this be a Qt specific problem? Which version of Qt have you tested
on?


> When using qFindChild, are we sure there is a unique direct child for an
> action ?
> Technically, there could be multiple ones, in which case the one being
> deleted
> is may still be addressed elsewhere.  (qt4 doc seems to say it is
> undefined in this case)
>

Whenever a single MenuItemData is created as a child of an action any
existing (one) is previously deleted. This happens in the same function, so
at any time, an action could logically have only one MenuItemData child.

In the same function, the new MenuItemData is set as mapping for it's parent
action on a QSignalMapper, and as there can be only one QObject mapped per
action (by Qt's documentation), I suppose that QSignalMapper's reference to
the deleted MenuItemData should vanish. Maybe this is not so on an older Qt
version. This is also the only possible reference to the deleted
MenuItemData that I see.


> As a matter of fact, just reverting this patch does solve the problem.
>
> Was this patch correcting a specific issue or just intended as a
> cosmetic ?
>

This patch targeted what I see as a real problem: unnecessary accumulation
of memory every time the Video, Audio and Interface menus are rebuilt =
shown again.

Please, check what version of Qt you are using, send valgrind's report if
possible, and try the attached patch that might solve the bug.

Best regards,

Jakob
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20100405/b901bfa9/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Qt-remove-any-previous-mappings-for-a-menu-action-be.patch
Type: text/x-diff
Size: 936 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20100405/b901bfa9/attachment.patch>