Jelmer,

I like the pidl restructuring you are doing as it makes the code much
cleaner, but I thought I should mention that your changes have broken
the pidl ethereal plugin generator. To see whats broken checkout the
"lorikeet" svn tree, edit ethereal/Makefile to point at the current
svn tree then run "make" in the ethereal subdirectory.

Compiling /home/tridge/samba/samba4/source/librpc/idl/security.idl
Undefined subroutine &util::is_scalar_type called at /data/samba/samba4/source/build/pidl/eparser.pm line 243, line 224.

I can see two obvious ways to fix this:

1) fix eparser.pm to use the new function names from ndr.pm

2) change eparser.pm to not use the "rewrite using regex" technique

The 2nd is the better long term change, but is _much_ more work.

One approach might be to change ndr.pm to generate code that uses
substitution templates to set things up for the right type of output.

For example we could have ndr_ethereal.tpl and ndr_samba.tpl as two
template files, which would contain things like this:

my $tmpl_hf_declare = "static int hf_%{NAME} = -1;\n"

Then ndr.pm would load the template, and do this:

template_set("PIPE" => "lsa");
template_set("NAME" => $thename);
pidl $hf_declare_tmpl

where the pidl function would be changed to always take a template. If
the template is empty (as it would be in ndr_samba.tpl in the above
example) then it does nothing. If it is non-empty then it does the
hash substitution and outputs the result;

The templating itself is pretty easy - there are plenty of perl
template systems around, or writing a new one is just a few lines of
code.

A slightly less trivial template might be:

my $tmpl_rqst = "
int %{PIPE}_%{FUNCTION}_rqst(tvbuff_t *tvb, int offset,
packet_info *pinfo, proto_tree *tree, guint8 *drep)
{
struct pidl_pull *ndr = pidl_pull_init(tvb, offset, pinfo, drep);
struct %{PIPE}_%{FUNCTION} *r = talloc_p(NULL, struct %{PIPE}_%{FUNCTION});
pidl_tree ptree;
ptree.proto_tree = tree;
ptree.subtree_list = NULL;
ndr_pull_%{PIPE}_%{FUNCTION}(ndr, NDR_IN, &ptree, r);
return ndr->offset;
}
";

which would generate all the _rqst() functions needed by ethereal.

Obviously we will need to break many functions up into separate
parts.

I'm not saying you should necessarily do this now, but I think that
long term this might be worth considering.

Cheers, Tridge