[PATCH v2 0/1] MR423: ieframe fix
Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> -- v2: ieframe: Fix memory leak and possible reference leak https://gitlab.winehq.org/wine/wine/-/merge_requests/423
From: David Kahurani <k.kahurani(a)gmail.com> Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> --- dlls/ieframe/iexplore.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dlls/ieframe/iexplore.c b/dlls/ieframe/iexplore.c index 1616d4d7f84..f1e1c5f7f60 100644 --- a/dlls/ieframe/iexplore.c +++ b/dlls/ieframe/iexplore.c @@ -1028,8 +1028,13 @@ static HDDEDATA open_dde_url(WCHAR *dde_url) hres = IWebBrowser2_Navigate2(&ie->IWebBrowser2_iface, &urlv, NULL, NULL, NULL, NULL); if(FAILED(hres)) + { + SysFreeString(V_BSTR(&urlv)); + IWebBrowser2_Release(&ie->IWebBrowser2_iface); return 0; + } + SysFreeString(V_BSTR(&urlv)); IWebBrowser2_Release(&ie->IWebBrowser2_iface); return ULongToHandle(DDE_FACK); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/423
Nikolay Sivov (@nsivov) commented about dlls/ieframe/iexplore.c:
hres = IWebBrowser2_Navigate2(&ie->IWebBrowser2_iface, &urlv, NULL, NULL, NULL, NULL); if(FAILED(hres)) + { + SysFreeString(V_BSTR(&urlv)); + IWebBrowser2_Release(&ie->IWebBrowser2_iface); return 0; + }
+ SysFreeString(V_BSTR(&urlv)); IWebBrowser2_Release(&ie->IWebBrowser2_iface); return ULongToHandle(DDE_FACK); }
VariantClear() seems more fitting. Also, you don't need this duplication. Just use FAILED(hres) to return 0 or DDE_FACK. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/423#note_3881
On Mon Jul 11 17:59:39 2022 +0000, Nikolay Sivov wrote:
VariantClear() seems more fitting. Also, you don't need this duplication. Just use FAILED(hres) to return 0 or DDE_FACK. Hello.
Thanks for the review. Yes, VariatClear() would indeed be more fitting but I used SysFreeString instead for consistency with the rest of the file. Should I ignore that? Or...maybe look into migrating the whole module? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/423#note_3882
On Mon Jul 11 18:30:06 2022 +0000, David Kahurani wrote:
Hello. Thanks for the review. Yes, VariatClear() would indeed be more fitting but I used SysFreeString instead for consistency with the rest of the file. Should I ignore that? Or...maybe look into migrating the whole module? Both VariantClear and SysFreeString seem fine to me here as long as it's not duplicated.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/423#note_3885
participants (4)
-
David Kahurani -
David Kahurani (@kahurani) -
Jacek Caban (@jacek) -
Nikolay Sivov (@nsivov)