I looked at it briefly:
A:
It looks good, but is packet-dcerpc-macros.h really needed?
I think it would be better if all the code in macros were to be implemented
as subroutines inside the .c file
instead. If they need to be visible outside packet-dcerpc-afs4int.c then
those functions can be exported.
So skip the huge code macros and replace them with functions taking these
parameters :
int func(tvbuff_t *tvb, int offset, packet_info *pinfo, proto_tree *tree,
char *drep);
the parameters are important so that we can, if we want to, call it through
a pointer.
I assume that in AFS4INT interface there are NO embedded pointers?
Well, anyway, I think it would be neater that after the change above
to make sure all these functions are called through a pointer, since there
are
pointers there but since they are top-level they are just not visible in the
bytestream.
So change the code then to call the functions above with the following
instead of directly:
offset=dissect_ndr_pointer(tvb, offset, pinfo, tree, drep,
func, NDR_POINTER_REF,
"func name", -1, 0);
B:
Below is something to consider/discuss for the future:
The packet-dcerpc-dce122.c file starts getting a very large value_string.
I think i remember something about dce-1.x placing some structure on this
32bit status/error value. Something like first 8 bits encode a ASCII char
identifying which subsystem generated the fault?
This part of the spec should be found again and then one should consider
1, replacing the single huge value_string array with one separate array for
each
subsystem.
2, add a function to decode the status/error message properly and make it
pick the proper
table automatically.
3, change the dce-122 dissectors to call the function in 2 whenever they
need to
dissect a rc/status value.
Apart from (A) it looks good. It should be very easy to implement (A)
using some simple emacs macros.
(B) is something one can implement sometime later, there may be better ways
to do it as well than what i suggested above.
best regards
ronnie sahlberg
Jaime Fournier wrote:
>As per the release early and often, I have decided to
>release my rewrite of packet-dcerpc-afs4int.c so I
>could get early feedback. I have released this before,
>but it was highly redundant, and incomplete. While
>this new version still lacks a couple of items, before
>it can be considered 100% complete. I wanted to get
>feedback on changes needed. Also included are the
>packet-dcerpc-macros.h, and a more comprehensive
>packet-dceprc-dce122.c.
>I will be working on completing the items not
>finished, marked with a XXX.
>My goal is to have all items of value in the dce_dfs
>level, to be presented on the COL_INFO line.
>Since I use tethereal in a packet logging mode.
>
>Thanks in advance!
>
>P.S. I am on the dev list, so there is no need to cc
>me on replies. Tha