Wireshark · Ethereal-dev: Re: [Ethereal-dev] small idl2eth update

[Bernd Becker <bb@xxxxxxxxxxxxxxx>]

Hi Guy and any GIOP users out there,

the fix to ethereal_gen.py from 1.16 to 1.17 broke the generated code
a little bit. First a small syntax error sneaked in,
col_clear(pinfo->cinfo, COL_INFO)) .
                                ^
But what this change actually does is delete the INFO column, because
nothing is inserted in the column in the subdissectors. Leaving it
untouched just keeps the info inserted by the GIOP dissector, which is
OK. So I am backing it out whith a comment that something useful should
be generated for the info column. I will try to come up with something.

The changes in packet-coseventcomm.c and packet-cosnaming.c
just reflect your change to ethereal_gen.py. After Frank Singleton
finishes his current work with ethereal_gen.py it might make
sense for him to regenerate the two dissectors from the IDL.

Additionally I have a small fix to packet-giop.c that resets the
COL_PROTOCOL to GIOP if none of the heuristic subdissectors
succeeded. Right now each subdissector is setting the COL_PROTOCOL.
If none is successful, the protocol name of the last one called
remains in the protocol column, which is wrong and confusing.

Actually, each subdissector adds a subtree to the protocol tree
as well. If the subdissector is not the correct one, the empty
subtree remains in the tree. If you have many subdissectors, this
is a bit annoying (although useful for debugging).

The correct solution to this would probably be to only set the
columns and insert the subtree in the protocol tree after
succesfull dissection. But this requires more work in the
subdissectors. Right now, the decision about dissecting
is made based only on the "operation". If the operation is
known to the subdissector, it goes ahead dissecting the rest
and returns TRUE, regardless of whether dissection was
really succesful. I have had cases where "similar" IDLs use
the same operation names, but the structure of the message is
quite different. So, if the "wrong" subdissector gets called
before the right one, it may end up throwing an exception.
The correct subdissector is never called. Right now this can
be solved by deactivating the "wrong" subdissectors in the
protocol table. Any ideas on how to best solve this?

Cheers,
Bernd

--On Montag, Dezember 17, 2001 14:32:11 -0800 Guy Harris <guy@xxxxxxxxxx> 
wrote:
> > One last patch before I take some XMAS holidays.
> >
> > idl2eth.diff - Patch for ethereal_gen.py to use the leaner "pinfo->cinfo"
> >                code in the template, instead of "pinfo->fd".
>
> Checked in (with additional changes to use "col_set_str()" rather than
> "col_add_str()", and to clear the Info column as well).
Index: ethereal_gen.py
===================================================================
RCS file: /cvsroot/ethereal/ethereal_gen.py,v
retrieving revision 1.18
diff -u -r1.18 ethereal_gen.py
--- ethereal_gen.py	2001/12/17 22:51:42	1.18
+++ ethereal_gen.py	2001/12/18 11:48:53
@@ -2114,8 +2114,13 @@
     if (check_col(pinfo->cinfo, COL_PROTOCOL))
        col_set_str(pinfo->cinfo, COL_PROTOCOL, \"@disprot@\");
 
-    if (check_col(pinfo->cinfo, COL_INFO))
-       col_clear(pinfo->cinfo, COL_INFO))
+/* 
+ * Do not clear COL_INFO, as nothing is being written there by 
+ * this dissector yet. So leave it as is from the GIOP dissector.
+ * TODO: add something useful to COL_INFO 
+ *  if (check_col(pinfo->cinfo, COL_INFO))
+ *     col_clear(pinfo->cinfo, COL_INFO);
+ */
 
     if (ptree) {
        ti = proto_tree_add_item(ptree, proto_@dissname@, tvb, *offset, tvb_length(tvb) - *offset, FALSE);
Index: packet-giop.c
===================================================================
RCS file: /cvsroot/ethereal/packet-giop.c,v
retrieving revision 1.52
diff -u -r1.52 packet-giop.c
--- packet-giop.c	2001/12/17 22:45:18	1.52
+++ packet-giop.c	2001/12/18 11:49:02
@@ -1762,6 +1762,9 @@
     } /* protocol_is_enabled */
   } /* loop */
 
+  if (check_col (pinfo->cinfo, COL_PROTOCOL))
+      col_set_str (pinfo->cinfo, COL_PROTOCOL, "GIOP");
+
   pinfo->current_proto = saved_proto;
   return res;			/* result */
 
Index: plugins/giop/packet-coseventcomm.c
===================================================================
RCS file: /cvsroot/ethereal/plugins/giop/packet-coseventcomm.c,v
retrieving revision 1.4
diff -u -r1.4 plugins/giop/packet-coseventcomm.c
--- plugins/giop/packet-coseventcomm.c	2001/12/17 22:50:27	1.4
+++ plugins/giop/packet-coseventcomm.c	2001/12/18 11:49:03
@@ -649,7 +649,7 @@
     guint32  offset_saved = (*offset);  /* save in case we must back out */
 
     if (check_col(pinfo->cinfo, COL_PROTOCOL))
-       col_add_str(pinfo->cinfo, COL_PROTOCOL, "COSEVENTCOMM");
+       col_set_str(pinfo->cinfo, COL_PROTOCOL, "COSEVENTCOMM");
 
     if (ptree) {
        ti = proto_tree_add_item(ptree, proto_coseventcomm, tvb, *offset, tvb_length(tvb) - *offset, FALSE);
Index: plugins/giop/packet-cosnaming.c
===================================================================
RCS file: /cvsroot/ethereal/plugins/giop/packet-cosnaming.c,v
retrieving revision 1.5
diff -u -r1.5 plugins/giop/packet-cosnaming.c
--- plugins/giop/packet-cosnaming.c	2001/12/17 22:50:27	1.5
+++ plugins/giop/packet-cosnaming.c	2001/12/18 11:49:05
@@ -1684,7 +1684,7 @@
     guint32  offset_saved = (*offset);  /* save in case we must back out */
 
     if (check_col(pinfo->cinfo, COL_PROTOCOL))
-       col_add_str(pinfo->cinfo, COL_PROTOCOL, "COSNAMING");
+       col_set_str(pinfo->cinfo, COL_PROTOCOL, "COSNAMING");
 
     if (ptree) {
        ti = proto_tree_add_item(ptree, proto_cosnaming, tvb, *offset, tvb_length(tvb) - *offset, FALSE);