MIDLC: The MIDL Compatible IDL Compiler

[Michael B Allen]

tridge at samba.org said:
>  > int
>  > lsarpc_LsarLookupSids(void *context,
>  >             const unsigned char *src,
>  >             const unsigned char *slim,
>  >             unsigned char *dst,
>  >             unsigned char *dlim)
>
> I know it's early days for MIDLC, but I hope you don't mind if I
> provide some critique of the interface.

How dare you! Just kidding. Actually MIDLC is pretty far along at this
point. We use the Java stubs and they work flawlessly.

> In the above, it really needs to use DATA_BLOB or similar, as you need
> range checking.

What do you mean by range checking? The lim sentinel pointers are passed
all the way down into the {enc,dec}_ndr_* primative functions (and
*everything* is ultimately encoded using those) so there should be no
overruns.

> Please, you need to pass in information on the byte
> order.

No problem. The void *context was thinking along those lines.

> The equivalent PIDL fn is this one:
>
>   NTSTATUS dcerpc_lsa_LookupSids(struct dcerpc_pipe *p,
>                                  TALLOC_CTX *mem_ctx,
>                                  struct lsa_LookupSids *r)
>
> the 'struct dcerpc_pipe' gives all the context info, the mem_ctx gives
> an allocation context, and the 'struct lsa_LookupSids' defines the
> parameters of the call.

No problem. This is top-level glue.

> Another thing to consider is async calls. PIDL generates a separate
> _send() function for every call, which allows just one side of the
> call to be made. This is essential for generating an async-enabled
> API.

Ok. I'm not sure what you ultimately want (two functions instead of one?)
but I don't think that should be a problem. I think we can just generate
an interface at which we can splice in the MIDLC generated code with
minimal changes to Samba. That's important so people can try it and
experiment while using PIDL at the same time. Then as we work out the
kinks and you gain more confidence in the code more interfaces can be
converted to slowly phase in MIDLC.

>  >     params.retval = LsarLookupSids(context,
>  >             params.handle,
>  >             params.sids,
>  >             params.domains,
>  >             params.names,
>  >             params.level,
>  >             params.count);
>
> this is the big problem. The LsarLookupSids() call in the above takes
> one C argument per parameter. That means that every RPC call takes a
> different number of parameters. That makes it impossible to use
> function pointers and generic driver code.

Oh, I thought the following qualified as "generic":

typedef int (*iface_Operation)(void *context,
           const unsigned char *src,
           const unsigned char *slim,
           unsigned char *dst,
           unsigned char *dlim)

> Also notice that this is what makes the 'validate' code in Samba4
> possible. If you look inside dcerpc_ndr_request_send() you will see
> checks for the DCERPC_DEBUG_VALIDATE_IN flag, in which case all
> marshalled data in unmarshalled then marshalled again and compared,
> which allows us to runtime select the validation of our NDR code. That
> would not be possible with MIDL.

Yikes. Unmarshall it, marshall it again and compare? Well I think we're
still going to have {enc,dec}_params_* functions so you'll have the
building blocks to do that.

> I know that MIDL does it the way you have done it, but it really is a
> very bad idea. We have gained a lot in the structure of Samba4 from
> the simple decision to use a structure (in this case 'struct
> lsa_LookupSids') rather than a MIDL like interface.
>
> Would you be willing to consider changing this in MIDLC ?

No problem. There is a function table that changes the emitter depending
on the -t <type> switch. Currently we have 'java' and 'c'. So we'll just
have add a 'samba' or something like that. That will trigger the emitter
for your samba specific call-glue. All C based emitters can still call the
core functions where the heavy lifting takes place.

>
>  >     enc_ndr_long(ndr, obj->length, dst, dlim);
>  >     enc_ndr_short(ndr, obj->impersonation_level, dst, dlim);
>  >     enc_ndr_small(ndr, obj->context_mode, dst, dlim);
>  >     enc_ndr_small(ndr, obj->effective_only, dst, dlim);
>
> obviously error checking is needed, as we don't have java exceptions :-)

Actually I was thinking about this and instead of checking return values
for every single little primative call I think it would be smoother to
just set an error code in the 'ndr' context that triggers all functions to
just quit and not advance dst at all. So the functions are called but
nothing get's encoded. And I can maybe use setjmp/longjmp too. Don't
worry. The marshalling routines will be safe.

> PIDL also generates a lot more than just the marshall/unmarshall
> functions. It generates the following per IDL function:
>
>  - the C structure header

Actually I think this is done already. Check the latest release.

>  - the _push function (mashshalling)

aka enc_params_Foo

>  - the _pull function (unmarshalling)

aka dec_params_Foo

>  - the _print function (essential for debugging)

I'll leave that as an exercise :->

>  - an optional _size function (essential for some IDL constructs)

Actually I don't think you want to try to compute the size of a structure
or object without actually just encoding it. There are some cases where
it's actually not possible. See the -v (verbose) output of MIDLC. It
prints the entire parse tree with precomputed sizes, alignments, and even
offsets.

>  - an entry in the interfaces function table

I don't know what that is but sure.

>  - the client stub

Don't have it. I think the top-level emit_client_stub functions are there
but they're empty. But fortunately the emit_{encoder,decoder}_fragment
functions (again, they do all the heavy lifting) are the same for client
and server stubs. So once we're certain the server stubs are working good
the client stubs are pretty straight forward.

>  - an async _send client stub
>  - the server stub

Well it's not crystal what you want so I'll just get as far as I can with
the call-glue I have now knowing that we're just going to add a -t samba
option and insert samba specific call-glue later. Again I can make it
match the interface you have now (whatever that is). Try and think of a
good place to splice in MIDLC stubs and let me know.

It might help to explain why I think the top-level stuff is easy. The
parser gives you a simple tree of 'struct sym *' that you just traverse
and printf whatever you need to emit. The sym struct has a linked list for
members (parameters for functions and members for structures and unions),
a hashmap for attributes, and a whole bunch of other stuff like name,
idl_type, ndr_type, size, align, etc. Consider the following emmitter code
that emits the code to encode a 'struct params_Xxx' structure (the
function parameters):

int
emit_params_encoder(struct idl *idl,
        struct sym *sym,
        int indent)
{
    iter_t iter;
    struct sym *mem, sym0;

    print(idl, indent, "int\nenc_params_%s(struct ndr *ndr,\n\t\t\tstruct
params_%s *obj\n\t\t\tunsigned char **dst,\n\t\t\tunsigned char
**deferred,\n\t\t\tunsigned char *dlim)\n{\n", sym->name, sym->name);

    linkedlist_iterate(&sym->mems, &iter);
    while ((mem = linkedlist_next(&sym->mems, &iter))) {
        if (hashmap_get(&mem->attrs, "out") == NULL) {
                     /* We're encoding so we're only
                      * interested in [out] members */
            continue;
        }

        sym0 = *mem; /* Copy the symbol so we can
                      * change stuff (e.g. the pointer
                      * level) */
        if (IS_REF(mem)) {
            sym0.ptr--;
        }
                     /* emit_encoder_fragment does the rest
                      * mode 0 means encode non-deferred stuff
                      * mode 1 means encode deferred stuff
                      * recursively
                      */
        if (emit_encoder_fragment(idl, &sym0, 0, indent + 4) == -1) {
            AMSG("");
            return -1;
        }
        if (emit_encoder_fragment(idl, &sym0, 1, indent + 4) == -1) {
            AMSG("");
            return -1;
        }
    }

    print(idl, indent + 4, "return 0;\n");
    print(idl, indent, "}\n");

    return 0;
}

This model is pretty flexible. You could write a function to generate
interface documentation, ethereal dissector code, XML ... whatever. It's
the emit_{decoder,encoder}_fragment fucntions that do the real work.
They're not a lot of code but if you look at them too long they will twist
your noodle into a pretzel!

Mike