Richard Sharpe wrote:
> ROUTINES
>
> It is expected that the following routines are needed:
>
> proto_t denc_lookup_protocol(char *name)
Where is proto_t used? [But see my comment about decode below.]
Presumably denc_lookup_protocol could be private (static) to the
implementation.
>
> int denc_register_protocol(char *name, parent_proto, proto_value,
> decode_routine, encode_routine);
proto_value should probably be a (void*)
>
> int denc_decode(char *pd, int offset, proto_node_t *tree[], int maxdepth)
The proto_node_t *tree[] declaration is confusing. This seems to imply
that you are returning an array of proto_node_t's and I cannot figure
out why. What would this return? Also, would the depth ever be anything
other than 0 or some-value-to-represent-infinity? How about?
proto_node_t *denc_decode(u_char *pd, int offset, unsigned flags)
[ node == NULL implies failure]
Unless you need the int result to indicate something other than
success/failure.
I suggest the flags so that the routine could be told to be recursive,
maybe do only a limited decode (e.g. for a summary display as in
Ethereal's top pane,
Then one could say:
node = denc_decode(data, 0, RECURSIVE | SUMMARY);
or
node = denc_decode(data, ETHER_DATA, RECURSIVE | FULL);
One thing I am unclear on, though: are you expecting a huristic on the
outermost protocol? E.g. consider my two examples above, in the first,
data could logically be Ethernet, TokenRing, etc and in the second IP,
IPX, etc. The problem is that in the second (or even the first), how
would the routine know whether to try IP, IPX, etc or Ethernet,
TokenRing, or even SPX, TCP, etc.? Is another parameter needed to tell
denc_decode what the top level protocol is assumed to be? This could be
done by specifying the name of the most encapsulating protocol as in:
node = denc_decode(data, ETHER_OFFSET, "ip", RECURSIVE|FULL);
>
> int denc_encode(char *pd, int offset, ...)
Thoughts?
--john
--
John McDermott jjm@xxxxxxxxxx
Writer and Computer Consultant
J-K International, Ltd.
+1 505/377-6293 - V
+1 505/377-6313 - F
