Re: [2/6] widl: Ignore assigment of a duplicate uuid.

[Huw Davies]

On Thu, Oct 15, 2015 at 08:46:46PM +0800, Dmitry Timoshkov wrote:
> MSDN KB285146 article describes this behaviour, and the tests in the last
> patch confirm that.

That article also mentions that the warning doesn't occur when the udt is
an enum (and indeed midl doesn't issue a warning in this case).  We should
do likewise.

[Also, while you're at it, please fix the typo in subject: 'assigment']

Huw.

[2/6] widl: Ignore assigment of a duplicate uuid.

[Dmitry Timoshkov]

Huw Davies  wrote:
> On Thu, Oct 15, 2015 at 08:46:46PM +0800, Dmitry Timoshkov wrote:
> > MSDN KB285146 article describes this behaviour, and the tests in the last
> > patch confirm that.
>
> That article also mentions that the warning doesn't occur when the udt is
> an enum (and indeed midl doesn't issue a warning in this case).  We should
> do likewise.

I think that it's just an omission on the midl side that it doesn't emit
a warning for an enum, because actually midl handles enums same way as unions
and structs: duplicate uuid is ignored. I think that a warning in that case
reflects real behaviour and should be printed.
> [Also, while you're at it, please fix the typo in subject: 'assigment']

Thanks for the review Huw.

[2/6] widl: Ignore assigment of a duplicate uuid.

[Huw Davies]

On Mon, Oct 26, 2015 at 07:02:57PM +0800, Dmitry Timoshkov wrote:
> Huw Davies  wrote:
> > On Thu, Oct 15, 2015 at 08:46:46PM +0800, Dmitry Timoshkov wrote:
> > > MSDN KB285146 article describes this behaviour, and the tests in the last
> > > patch confirm that.
> >
> > That article also mentions that the warning doesn't occur when the udt is
> > an enum (and indeed midl doesn't issue a warning in this case).  We should
> > do likewise.
>
> I think that it's just an omission on the midl side that it doesn't emit
> a warning for an enum, because actually midl handles enums same way as unions
> and structs: duplicate uuid is ignored. I think that a warning in that case
> reflects real behaviour and should be printed.

I guess I don't care too much on this point.

However, we are getting these warnings during a Wine build which I think
should be investigated / silenced before this patch is committed.

../tools/widl/widl -o stdole2.tlb -m32 -I. -I../include -D__WINESRC__ stdole2.idl
warning: duplicate uuid {00020430-0000-0000-c000-000000000046}
warning: duplicate uuid {00020400-0000-0000-c000-000000000046}

These are most likely due to the add_dispatch() stuff, so we should probably
just silience the warning for these uuids.

../../tools/widl/widl -o msxml3_v1_t.res -m32 -I. -I../../include -I/usr/include/libxml2 -D__WINESRC__ \
  msxml3_v1.idl
warning: duplicate uuid {50ea08b4-dd1b-4664-9a50-c2f40f4bd79a}
(plus the same warnings in other msxml* directories).

This is ISchema.  Not sure why this is happening.

../../tools/widl/widl -o winhttp_tlb_t.res -m32 -I. -I../../include -D__WINESRC__ winhttp_tlb.idl
warning: duplicate uuid {12782009-fe90-4877-9730-e5e183669b19}
warning: duplicate uuid {9d8a6df8-13de-4b1f-a330-67c719d62514}

These are enums and so would go away if we followed midl.

Huw.

[2/6] widl: Ignore assigment of a duplicate uuid.

[Dmitry Timoshkov]

Huw Davies  wrote:
> ../../tools/widl/widl -o msxml3_v1_t.res -m32 -I. -I../../include -I/usr/include/libxml2 -D__WINESRC__ \
>   msxml3_v1.idl
> warning: duplicate uuid {50ea08b4-dd1b-4664-9a50-c2f40f4bd79a}
> (plus the same warnings in other msxml* directories).
>
> This is ISchema.  Not sure why this is happening.

The warning is emitted because the hash keys returned by ctl2_hash_guid()
for IXMLDOMSchemaCollection2 (50ea08b0-dd1b-4664-9a50-c2f40f4bd79a) and
ISchema (50ea08b4-dd1b-4664-9a50-c2f40f4bd79a) are the same. It seems to
me a bit inadequate that ctl2_hash_guid() does 'return hash & 0x1f'. But
I think that fixing this problem is outside of the scope of my patches
and deserves a separate investigation.

[2/6] widl: Ignore assigment of a duplicate uuid.

[Alexandre Julliard]

Dmitry Timoshkov  writes:
> Huw Davies  wrote:
> > ../../tools/widl/widl -o msxml3_v1_t.res -m32 -I. -I../../include -I/usr/include/libxml2 -D__WINESRC__ \
> >   msxml3_v1.idl
> > warning: duplicate uuid {50ea08b4-dd1b-4664-9a50-c2f40f4bd79a}
> > (plus the same warnings in other msxml* directories).
> >
> > This is ISchema.  Not sure why this is happening.
>
> The warning is emitted because the hash keys returned by ctl2_hash_guid()
> for IXMLDOMSchemaCollection2 (50ea08b0-dd1b-4664-9a50-c2f40f4bd79a) and
> ISchema (50ea08b4-dd1b-4664-9a50-c2f40f4bd79a) are the same. It seems to
> me a bit inadequate that ctl2_hash_guid() does 'return hash & 0x1f'. But
> I think that fixing this problem is outside of the scope of my patches
> and deserves a separate investigation.

If a hash collision triggers a warning that would definitely be a bug in
your code, but I don't think that's the reason here.

[2/6] widl: Ignore assigment of a duplicate uuid.

[Dmitry Timoshkov]

Alexandre Julliard  wrote:
> > > ../../tools/widl/widl -o msxml3_v1_t.res -m32 -I. -I../../include -I/usr/include/libxml2 -D__WINESRC__ \
> > >   msxml3_v1.idl
> > > warning: duplicate uuid {50ea08b4-dd1b-4664-9a50-c2f40f4bd79a}
> > > (plus the same warnings in other msxml* directories).
> > >
> > > This is ISchema.  Not sure why this is happening.
> >
> > The warning is emitted because the hash keys returned by ctl2_hash_guid()
> > for IXMLDOMSchemaCollection2 (50ea08b0-dd1b-4664-9a50-c2f40f4bd79a) and
> > ISchema (50ea08b4-dd1b-4664-9a50-c2f40f4bd79a) are the same. It seems to
> > me a bit inadequate that ctl2_hash_guid() does 'return hash & 0x1f'. But
> > I think that fixing this problem is outside of the scope of my patches
> > and deserves a separate investigation.
>
> If a hash collision triggers a warning that would definitely be a bug in
> your code, but I don't think that's the reason here.

Do you have an idea what might be a reason of the collision?

[2/6] widl: Ignore assigment of a duplicate uuid.

[Nikolay Sivov]

On 27.10.2015 10:08, Dmitry Timoshkov wrote:
> Alexandre Julliard  wrote:
> > > > ../../tools/widl/widl -o msxml3_v1_t.res -m32 -I. -I../../include -I/usr/include/libxml2 -D__WINESRC__ \
> > > >    msxml3_v1.idl
> > > > warning: duplicate uuid {50ea08b4-dd1b-4664-9a50-c2f40f4bd79a}
> > > > (plus the same warnings in other msxml* directories).
> > > >
> > > > This is ISchema.  Not sure why this is happening.
> > >
> > > The warning is emitted because the hash keys returned by ctl2_hash_guid()
> > > for IXMLDOMSchemaCollection2 (50ea08b0-dd1b-4664-9a50-c2f40f4bd79a) and
> > > ISchema (50ea08b4-dd1b-4664-9a50-c2f40f4bd79a) are the same. It seems to
> > > me a bit inadequate that ctl2_hash_guid() does 'return hash & 0x1f'. But
> > > I think that fixing this problem is outside of the scope of my patches
> > > and deserves a separate investigation.
> >
> > If a hash collision triggers a warning that would definitely be a bug in
> > your code, but I don't think that's the reason here.
>
> Do you have an idea what might be a reason of the collision?

Collision on its own is not a problem. ctl2_hash_guid() returns index, 
not a hash value, and GUID hash segment size is fixed to 0x80 (0x80 / 
0x1f = sizeof(int)), so masking with 0x1f is adequate.

[2/6] widl: Ignore assigment of a duplicate uuid.

[Dmitry Timoshkov]

Nikolay Sivov  wrote:
> > > > > ../../tools/widl/widl -o msxml3_v1_t.res -m32 -I. -I../../include -I/usr/include/libxml2 -D__WINESRC__ \
> > > > >    msxml3_v1.idl
> > > > > warning: duplicate uuid {50ea08b4-dd1b-4664-9a50-c2f40f4bd79a}
> > > > > (plus the same warnings in other msxml* directories).
> > > > >
> > > > > This is ISchema.  Not sure why this is happening.
> > > >
> > > > The warning is emitted because the hash keys returned by ctl2_hash_guid()
> > > > for IXMLDOMSchemaCollection2 (50ea08b0-dd1b-4664-9a50-c2f40f4bd79a) and
> > > > ISchema (50ea08b4-dd1b-4664-9a50-c2f40f4bd79a) are the same. It seems to
> > > > me a bit inadequate that ctl2_hash_guid() does 'return hash & 0x1f'. But
> > > > I think that fixing this problem is outside of the scope of my patches
> > > > and deserves a separate investigation.
> > >
> > > If a hash collision triggers a warning that would definitely be a bug in
> > > your code, but I don't think that's the reason here.
> >
> > Do you have an idea what might be a reason of the collision?
>
> Collision on its own is not a problem. ctl2_hash_guid() returns index, 
> not a hash value, and GUID hash segment size is fixed to 0x80 (0x80 / 
> 0x1f = sizeof(int)), so masking with 0x1f is adequate.

Then ctl2_find_guid() shouldn't return same offset for different guids.

[2/6] widl: Ignore assigment of a duplicate uuid.

[Dmitry Timoshkov]

Dmitry Timoshkov  wrote:
> Then ctl2_find_guid() shouldn't return same offset for different guids.

Sorry, wrong wording, should be: Then ctl2_find_guid() shouldn't find the guid
and return -1.

[2/6] widl: Ignore assigment of a duplicate uuid.

[Alexandre Julliard]

Dmitry Timoshkov  writes:
> Nikolay Sivov  wrote:
> > > > > > ../../tools/widl/widl -o msxml3_v1_t.res -m32 -I. -I../../include -I/usr/include/libxml2 -D__WINESRC__ \
> > > > > >    msxml3_v1.idl
> > > > > > warning: duplicate uuid {50ea08b4-dd1b-4664-9a50-c2f40f4bd79a}
> > > > > > (plus the same warnings in other msxml* directories).
> > > > > >
> > > > > > This is ISchema.  Not sure why this is happening.
> > > > >
> > > > > The warning is emitted because the hash keys returned by ctl2_hash_guid()
> > > > > for IXMLDOMSchemaCollection2 (50ea08b0-dd1b-4664-9a50-c2f40f4bd79a) and
> > > > > ISchema (50ea08b4-dd1b-4664-9a50-c2f40f4bd79a) are the same. It seems to
> > > > > me a bit inadequate that ctl2_hash_guid() does 'return hash & 0x1f'. But
> > > > > I think that fixing this problem is outside of the scope of my patches
> > > > > and deserves a separate investigation.
> > > >
> > > > If a hash collision triggers a warning that would definitely be a bug in
> > > > your code, but I don't think that's the reason here.
> > >
> > > Do you have an idea what might be a reason of the collision?
> >
> > Collision on its own is not a problem. ctl2_hash_guid() returns index, 
> > not a hash value, and GUID hash segment size is fixed to 0x80 (0x80 / 
> > 0x1f = sizeof(int)), so masking with 0x1f is adequate.
>
> Then ctl2_find_guid() shouldn't return same offset for different guids.

It doesn't. That's not where the bug is.

[2/6] widl: Ignore assigment of a duplicate uuid.

[Dmitry Timoshkov]

Alexandre Julliard  wrote:
> > > > > > > ../../tools/widl/widl -o msxml3_v1_t.res -m32 -I. -I../../include -I/usr/include/libxml2 -D__WINESRC__ \
> > > > > > >    msxml3_v1.idl
> > > > > > > warning: duplicate uuid {50ea08b4-dd1b-4664-9a50-c2f40f4bd79a}
> > > > > > > (plus the same warnings in other msxml* directories).
> > > > > > >
> > > > > > > This is ISchema.  Not sure why this is happening.
> > > > > >
> > > > > > The warning is emitted because the hash keys returned by ctl2_hash_guid()
> > > > > > for IXMLDOMSchemaCollection2 (50ea08b0-dd1b-4664-9a50-c2f40f4bd79a) and
> > > > > > ISchema (50ea08b4-dd1b-4664-9a50-c2f40f4bd79a) are the same. It seems to
> > > > > > me a bit inadequate that ctl2_hash_guid() does 'return hash & 0x1f'. But
> > > > > > I think that fixing this problem is outside of the scope of my patches
> > > > > > and deserves a separate investigation.
> > > > >
> > > > > If a hash collision triggers a warning that would definitely be a bug in
> > > > > your code, but I don't think that's the reason here.
> > > >
> > > > Do you have an idea what might be a reason of the collision?
> > >
> > > Collision on its own is not a problem. ctl2_hash_guid() returns index, 
> > > not a hash value, and GUID hash segment size is fixed to 0x80 (0x80 / 
> > > 0x1f = sizeof(int)), so masking with 0x1f is adequate.
> >
> > Then ctl2_find_guid() shouldn't return same offset for different guids.
>
> It doesn't. That's not where the bug is.

Probably I'm missing something obvious, but I couldn't figure out the reason
of an attempt of creation a duplicate uuid entry.