Use of uninitialized variable in combine_uri()

[Gerald Pfeifer]

Hi Thomas,

the following change of yours 

  commit bced2e21dbc548ef9d41e3ff11384d7ad964c929
  Author: Thomas Mullaly <thomas.mullaly(a)gmail.com>
  Date:   Sat Oct 9 11:02:17 2010 -0400

    urlmon: Implemented base case for CoInternetCombineIUri.

introduces a new warning, use of uninitialized variable in the line
marked "HERE" below.

  +static HRESULT combine_uri(Uri *base, Uri *relative, DWORD flags, IUri **result
  +    Uri *ret;
  +    HRESULT hr;
  +    parse_data data;
  +
  +    /* Base case is when the relative Uri has a scheme name,
  +     * if it does, then 'result' will contain the same data
  +     * as the relative Uri.
  +     */
  +    if(relative->scheme_start > -1) {
  +        DWORD create_flags = 0;
  +
  +        memset(&data, 0, sizeof(parse_data));
  +
  +        data.uri = SysAllocString(relative->raw_uri);
  +        if(!data.uri) {
  +            IUri_Release(URI(ret)); <================== HERE
  +            *result = NULL;
  +            return E_OUTOFMEMORY;
  +        }
> From all I can tell this is a legitimate warning, that is, the code
> really invokes undefined behavior.  Would you mind having a look?

Gerald

[Thomas Mullaly]

Hi Gerald,

On Tue, Nov 2, 2010 at 6:51 PM, Gerald Pfeifer <gerald(a)pfeifer.com> wrote:
> Hi Thomas,
>
> the following change of yours
>
>  commit bced2e21dbc548ef9d41e3ff11384d7ad964c929
>  Author: Thomas Mullaly <thomas.mullaly(a)gmail.com>
>  Date:   Sat Oct 9 11:02:17 2010 -0400
>
>    urlmon: Implemented base case for CoInternetCombineIUri.
>
> introduces a new warning, use of uninitialized variable in the line
> marked "HERE" below.
>
>  +static HRESULT combine_uri(Uri *base, Uri *relative, DWORD flags, IUri **result
>  +    Uri *ret;
>  +    HRESULT hr;
>  +    parse_data data;
>  +
>  +    /* Base case is when the relative Uri has a scheme name,
>  +     * if it does, then 'result' will contain the same data
>  +     * as the relative Uri.
>  +     */
>  +    if(relative->scheme_start > -1) {
>  +        DWORD create_flags = 0;
>  +
>  +        memset(&data, 0, sizeof(parse_data));
>  +
>  +        data.uri = SysAllocString(relative->raw_uri);
>  +        if(!data.uri) {
>  +            IUri_Release(URI(ret)); <================== HERE
>  +            *result = NULL;
>  +            return E_OUTOFMEMORY;
>  +        }
>
> From all I can tell this is a legitimate warning, that is, the code
> really invokes undefined behavior.  Would you mind having a look?
>
> Gerald

Whoa! Good catch, I'll submit a new patch set here in a few minutes fixing that.

Thank you for the heads up.

-- 
Thomas Mullaly
thomas.mullaly(a)gmail.com