[Patch] fix IDL hang V2

Noel Power nopower at suse.com
Thu Aug 21 13:52:11 MDT 2014


On 21/08/14 17:34, Jelmer Vernooij wrote:
> On Thu, Aug 21, 2014 at 12:01:28PM +0100, Noel Power wrote:
>> On 07/08/14 12:22, Noel Power wrote:
>>> Hi,
>>>
>>> issuing
>>>
>>> ./pidl/pidl --header --ndr-parser -- foo.idl
>>>
>>> will cause pidl to hang, the attached patch prevents the recursion that
>>> triggers that. Note: I compared the contents of
>>> bin/default/librpc/gen_ndr before and after the patch and the contents
>>> are identical
>>>
>>> Noel
>> Note previous version had a bug, attached in a newer version, and also
>> less error-filled example idl(s) ( one that can be used with midl, one
>> with pidl). As previously mentioned it appears such a recursive
>> structure arrangement seems to cause MIDL to barf [1], I still don't
>> believe that is a reason for pidl to similarly fail (or hang), note:
>> also the ndr marshall/unmarshall generated work fine
> I'm wary of supporting constructions that are pidl-specific. We'd like to
> make our IDL files more compatible with midl, not less.
That's not at all clear from the current idl that seems littered with
pidl specific extensions. The fact that those specific properties aid
the the generation of standard marshall/unmarshall routines is imho a
*good* thing and should be encouraged, hand coding marshall/unmarshall
routines should be discouraged.
Support for compatible IDL files is something that may also be desirable
but is something that should be enforced by presence (or absence) of
some compatibility switch. but... I think there will always be a market
for the ability to generate more exotic messages from a description (idl
or otherwise) and thus I believe there will always be a place for those
non-conforming (from a midl pov) IDL files that already exist in the tree.
>
> Please add tests for new features (see t/ under the pidl/ directory).
is it a new feature to prevent a hang? not at all sure how to write a
test for a hang (note: when it hangs on my machine it quickly makes the
machine unusable) :-/ but I can try to write a test to see if the
compile succeeds if that helps...
>
> How does this handle nested struct without pointers? Does it error out?
>
> struct foo {
>   struct bar a;
>   hyper t;
> }; 
>
> struct bar {
>   struct foo b;
> }
>

not sure what you are getting at with the above, the structure above (if
converted into idl )

interface hang
{
typedef [public] struct {
bar a;
hyper t;
} foo ;

typedef [public] struct {
foo b;
} bar;
}


will hang if compiled with pidl without the patch, with patched pidl it
will succeed but it will result in generated code that rightly won't
compile "error: field ‘a’ has incomplete type", however using a pointer

interface hang
{
typedef [public] struct {
bar *a;
hyper t;
} foo ;

typedef [public] struct {
foo b;
} bar;
}

and the generated code is compilable, I don't see the problem.


Noel


More information about the samba-technical mailing list