On 07/09/12 17:49, Erich E. Hoover wrote:
On Mon, Jul 9, 2012 at 3:31 AM, Jacek Caban jacek@codeweavers.com wrote:
... How is it supposed to work? AFAICS you're destroying windows, removing info object from the list and forget about it. If your intention is to destroy these objects, then ReleaseHelpViewer seems more appropriate. Also moving list_remove call to ReleaseHelpViewer seems preferable, so that it's removed from the list regardless of the reason for being destroyed. Same for list_add_tail, shouldn't it be done in CreateHelpViewer?
That makes sense, I'll correct this and resubmit. Is there anything else you would like me to change?
Other patches will have to be slightly changed due to above, to their full review is not possible ATM. From what I see in patch 5, you also call list_add_tail outside constructor. I'd have also minor suggestions to use an inline function instead of NAVIGATION_VISIBLE macro and consider using LIST_FOR_EACH_ENTRY instead of LIST_FOR_EACH.
Jacek