[prev in list] [next in list] [prev in thread] [next in thread] 

List:       wine-devel
Subject:    Re: mshtml: Avoid memory leaks (coverity)
From:       Jacek Caban <jacek () codeweavers ! com>
Date:       2013-11-30 14:30:04
Message-ID: 5299F66C.7070902 () codeweavers ! com
[Download RAW message or body]

Hi Frédéric,

On 11/30/13 15:16, Frédéric Delanoy wrote:
> CID 1131417
> ---
>  dlls/mshtml/htmlbody.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/dlls/mshtml/htmlbody.c b/dlls/mshtml/htmlbody.c
> index fcd8766..163f0ba 100644
> --- a/dlls/mshtml/htmlbody.c
> +++ b/dlls/mshtml/htmlbody.c
> @@ -648,7 +648,8 @@ static HRESULT WINAPI HTMLBodyElement_get_scroll(IHTMLBodyElement *iface, BSTR *
>  
>      if(!overflow || !*overflow) {
>          *p = NULL;
> -        return S_OK;
> +        hres = S_OK;
> +        goto cleanup;
>      }else if(!strcmpW(overflow, visibleW) || !strcmpW(overflow, autoW)) {
>          ret = autoW;
>      }else if(!strcmpW(overflow, scrollW)) {
> @@ -658,11 +659,16 @@ static HRESULT WINAPI HTMLBodyElement_get_scroll(IHTMLBodyElement *iface, BSTR *
>      }else {
>          TRACE("Defaulting %s to NULL\n", debugstr_w(overflow));
>          *p = NULL;
> -        return S_OK;
> +        hres = S_OK;
> +        goto cleanup;
>      }
>  
>      *p = SysAllocString(ret);
> -    return *p ? S_OK : E_OUTOFMEMORY;
> +    hres = *p ? S_OK : E_OUTOFMEMORY;
> +
> +cleanup:
> +    SysFreeString(overflow);
> +    return hres;
>  }
>

Good catch, but those gotos make the code more complicated that it needs
to be. You may just remove early returns and free overflow before
allocating the result.

Jacek


[prev in list] [next in list] [prev in thread] [next in thread]