Re: [ros-dev] [ros-diffs] [ekohl] 53469: [WIDL] Revert parts of r53171: Remove the -Oif flag for WIDL because the NT4 stub code generated by the current WIDL is heavily broken with respect to the range attribute and others....

[Jérôme Gardou]

Please see inlined answers.

Le 28/08/2011 00:29, Eric Kohl a écrit :
>   Hello Jérôme!
>
> >   I'm not sure about what you mean for range
> > attribute, but I notice that
> >  typedef [context_handle] PVOID my_handle;
> >  makes widl thinks that any value of the my_handle type is passed by
> >  pointer, whereas midl considers it is passed by value. 
>  Did you try to find the bug
> in WIDL? 
In fact, it was our bug, fixed in r53477.
>  
> >   Could you be more precise about "range
> > attributes and others?" please. 
>  I should better have written "range
> attributes and probably others"!
>
>  The range attribute is used in the svcctl.idl file and enables range 
>  checks in rpcrt4.dll. This is a nice feature, but WIDL is messing up 
>  the ranges if a range attribute is used with function parameters.
>
>  For example:
>  DWORD Test(
>      [in] SC_RPC_HANDLE hSCManager,
>      [in, range(0, 1024)] DWORD dwParam1,
>      [in, range(0, 255)] DWORD dwParam2);
>
>  WIDL will emit the same range type (range 0-255) for both parameters 
>  because the range attribute is assigned to the DWORD type and it is 
>  overwritten whenever a new range attribute is used with the DWORD type.
>  In the end all DWORD type parameters that have a range attribute will 
>  use the range that is used by the last DWORD type with a range 
>  attribute (dwParam2 in this case) in the idl file. This will cause 
>  some 'funny' effects if dwParam1 is greater than 255. :-/ 
OK, I guess
we'd have to open a bug on wine side, I'm not comfortable 
enough with WIDL code to fix it myself.
>   BTW, I think it is more important to have a working
> RPC system in 
>  ReactOS than a windows-compatible one. The compatibility issues can be 
>  fixed later, but if you break RPC today by fixing compatibility issues 
>  you will render ReactOS unusable. So please be very careful when you 
>  make changes to rpcrt4 or WIDL. 
I fully agree with that, but just try to reenable
-Oif, and look at 
advapi32:service winetest results. You'd be surprised : no skipped 
tests, and some others are actually fixed! (while others are broken, 
most probably because of this range thing) So it's more than a "nice 
feature". You can already see it fixes one eventlog test by looking at 
the link Olaf provided (look for eventlog.c:206).
>  
>  Regards,
>  Eric
>
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev 
Regards.
Jérôme.