Re: [PATCH] urlmon: set hres also in else branch (Coverity)

[Nikolay Sivov]

On 19.04.2015 14:42, Marcus Meissner wrote:
> 718705 Unchecked return value
>
> dlls/urlmon/session.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/dlls/urlmon/session.c b/dlls/urlmon/session.c
> index e01e647..001158f 100644
> --- a/dlls/urlmon/session.c
> +++ b/dlls/urlmon/session.c
> @@ -213,7 +213,7 @@ IInternetProtocolInfo *get_protocol_info(LPCWSTR url)
>   hres = IClassFactory_QueryInterface(cf, &IID_IInternetProtocolInfo, (void**)&ret);
>   if(FAILED(hres))
> -
>    IClassFactory_CreateInstance(cf, NULL, &IID_IInternetProtocolInfo, (void**)&ret);
> -
>    hres = IClassFactory_CreateInstance(cf, NULL, &IID_IInternetProtocolInfo, (void**)&ret);
> IClassFactory_Release(cf);
>
> return ret;

It's still unchecked. If we want to use it we need to propagate return 
code to caller, right now it will return NULL interface pointer. So I'm 
thinking it's false positive.

[PATCH] urlmon: set hres also in else branch (Coverity)

[Marcus Meissner]

On Sun, Apr 19, 2015 at 02:47:36PM +0300, Nikolay Sivov wrote:
> On 19.04.2015 14:42, Marcus Meissner wrote:
> > 718705 Unchecked return value
> >
> > dlls/urlmon/session.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/dlls/urlmon/session.c b/dlls/urlmon/session.c
> > index e01e647..001158f 100644
> > --- a/dlls/urlmon/session.c
> > +++ b/dlls/urlmon/session.c
> > @@ -213,7 +213,7 @@ IInternetProtocolInfo *get_protocol_info(LPCWSTR url)
> >  hres = IClassFactory_QueryInterface(cf, &IID_IInternetProtocolInfo, (void**)&ret);
> >  if(FAILED(hres))
> > -
> >    IClassFactory_CreateInstance(cf, NULL, &IID_IInternetProtocolInfo, (void**)&ret);
> > -
> >    hres = IClassFactory_CreateInstance(cf, NULL, &IID_IInternetProtocolInfo, (void**)&ret);
> >
> > IClassFactory_Release(cf);
> >
> > return ret;
>
> It's still unchecked. If we want to use it we need to propagate
> return code to caller, right now it will return NULL interface
> pointer. So I'm thinking it's false positive.

Yes, you are right. "ret" will be NULL on error anyway, so getting hres is not needed.

CIao, Marcus