"Pia Sahlberg" <piabar@xxxxxxxxxxx> writes:
> Hi,
>
> Attahced is a small patch for dcerpc and dcerpc-spoolss
> It is messy and just a test before the real work is done.
> [...]
> Also please comment on how the data is displayed.
>
Some comments:
- I'm not sure I see the point of the spoolss.cmd field being added.
It should already be available as dcerpc.opnum, no? And it doesn't
seem necessary to pass it down specially. It's available to the
subdissector implicitly via whichever subdissector function gets
called. E.g., you could instead do
print_spoolss_cmd (tvb, pinfo, tree, SPOOLSS_CLOSEPRINTER);
or
print_spoolss_cmd (tvb, pinfo, tree, SPOOLSS_GETPRINTERDATA);
or ...
Though if you think all subdissectors are going to want to have
something similar, then the dcerpc layer can probably do it itself.
But again, I don't see the rationale at this point. Why do you
need a spoolss.cmd?
- Generally, you should use the dissect_ndr_* functions instead of the
dissect_dcerpc_* functions. The NDR functions will ensure proper
alignment, etc. Of course, not everything you need is currently
available... so
- Instead of dissect_msrpc_POLICY_HND, how about using a
dissect_ndr_ctx_hnd function? I've been doing a little work on the
samr dissector (sending in a separate msg), and defined that in the
process. It doesn't add hf_index fields for all of the individual
fields within it, but I'm not sure we really want to.
- for unicode_to_str and dissect_msrpc_UNISTR2, I don't claim to
understand what you're doing :), but here are some comments. You're
assuming that the data on the wire is little-endian. That's not a
valid assumption. You have to check drep. I think the right way to
handle this is to define a dissect_ndr_conformant_varying_array, which
would handle the ndr stuff (len, endianness, etc.) and provide to its
caller the array of unsigned shorts. Hmmm, this is getting harder to
explain. Code would be much easier to understand, I think. SAMR uses
the same data structure, so I'll code up how I think it should work
there (in a day or two), and we can compare notes.
- lastly, on terminology, please let's stop using 'msrpc'. There's
really only dcerpc, NDR, and then some MS (or WINNT) specific data
structures defined on NDR.
Todd
