Re: [PATCH 4/6] include: Added definitions used by OpenFileById. (try 2)

[Dmitry Timoshkov]

"Vincas Miliūnas"  wrote:
> - Renamed DUMMYUNIONNAME to u.

...
> +typedef struct _FILE_ID_DESCRIPTOR {
> - DWORD        dwSize;
> - FILE_ID_TYPE Type;
> - union {
> -
>    LARGE_INTEGER FileId;
> -
>    GUID          ObjectId;
> - } u;
>
> +} FILE_ID_DESCRIPTOR, *LPFILE_ID_DESCRIPTOR;

This change is clearly wrong.

[PATCH 4/6] include: Added definitions used by OpenFileById. (try 2)

[Vincas Miliūnas]

On 10/11/2012 04:30 AM, Dmitry Timoshkov wrote:
> "Vincas Miliūnas"  wrote:
> > - Renamed DUMMYUNIONNAME to u.
>
> ...
> > +typedef struct _FILE_ID_DESCRIPTOR {
> > - DWORD        dwSize;
> > - FILE_ID_TYPE Type;
> > - union {
> > -
> >    LARGE_INTEGER FileId;
> > -
> >    GUID          ObjectId;
> > - } u;
> >
> > +} FILE_ID_DESCRIPTOR, *LPFILE_ID_DESCRIPTOR;
>
> This change is clearly wrong.

As far as I know WINE does not use anonymous unions to preserve
compatibility with compilers that do not support them. For the unions
that need to be accessed in the code, letter u is used to name them. git
grep '.u.' | wc -l returns 2523 lines of such usage to access union
members.

Of course the union can be removed and replaced by a single FileId
field. Also an issue might be that WinBase.h does not follow the
standard practice and defines struct FILE_ID_DESCRIPTOR instead of the
struct _FILE_ID_DESCRIPTOR like I submitted.

Unless you mind sharing why it is wrong, I will not be able to read your
mind.

Thanks for feedback.

[PATCH 4/6] include: Added definitions used by OpenFileById. (try 2)

[Dmitry Timoshkov]

"Vincas Miliūnas"  wrote:
> Unless you mind sharing why it is wrong, I will not be able to read your
> mind.

Just do your homework and have a look at other places that use DUMMYUNIONNAME.

[PATCH 4/6] include: Added definitions used by OpenFileById. (try 2)

[Vincas Miliūnas]

On 10/11/2012 07:29 PM, Dmitry Timoshkov wrote:
> "Vincas Miliūnas"  wrote:
> > Unless you mind sharing why it is wrong, I will not be able to read your
> > mind.
>
> Just do your homework and have a look at other places that use DUMMYUNIONNAME.

Thanks for sharing knowledge with your attitude.

[PATCH 4/6] include: Added definitions used by OpenFileById. (try 2)

[Dmitry Timoshkov]

"Vincas Miliūnas"  wrote:
> > > Unless you mind sharing why it is wrong, I will not be able to read your
> > > mind.
> >
> > Just do your homework and have a look at other places that use DUMMYUNIONNAME.
>
> Thanks for sharing knowledge with your attitude.

You started working on a complex API set implementation, and if you can't figure
this very trivial thing out that makes the whole approach pretty questionable.

[PATCH 4/6] include: Added definitions used by OpenFileById. (try 2)

[Francois Gouget]

On Thu, 11 Oct 2012, Vincas Miliūnas wrote:
> On 10/11/2012 04:30 AM, Dmitry Timoshkov wrote:
> > "Vincas Miliūnas"  wrote:
> > > - Renamed DUMMYUNIONNAME to u.
> >
> > ...
> > > +typedef struct _FILE_ID_DESCRIPTOR {
> > > - DWORD        dwSize;
> > > - FILE_ID_TYPE Type;
> > > - union {
> > > -
> > >    LARGE_INTEGER FileId;
> > > -
> > >    GUID          ObjectId;
> > > - } u;
> > >
> > > +} FILE_ID_DESCRIPTOR, *LPFILE_ID_DESCRIPTOR;
> >
> > This change is clearly wrong.
>
> As far as I know WINE does not use anonymous unions to preserve
> compatibility with compilers that do not support them. For the unions
> that need to be accessed in the code, letter u is used to name them. git
> grep '.u.' | wc -l returns 2523 lines of such usage to access union
> members.
>
> Of course the union can be removed and replaced by a single FileId
> field. Also an issue might be that WinBase.h does not follow the
> standard practice and defines struct FILE_ID_DESCRIPTOR instead of the
> struct _FILE_ID_DESCRIPTOR like I submitted.
>
> Unless you mind sharing why it is wrong, I will not be able to read your
> mind.

It's wrong because the PSDK headers use DUMMYUNIONNAME. It is then 
either defined to 'u' or an empty string as appropriate and that's what 
we do in Wine too. Check its definition in include/winnt.h. You may also 
want to check the use of NONAMELESSUNION in Wine.

You could also look a the git log --grep='support nameless' commits, for 
instance fe6451e2 and ae3a919b.