[Guy Harris <gharris@xxxxxxxxxxxx>]

> This code is based upon the assumption that a guint32 and gint are the same
> size.
> This assumption has been tested and found to be valid on my linux system.
> If that assumption is not correct, there are two choices: change the
> 'pattern' value
> to guint16 or the register_dissector routine must be changed replace the
> 'g_direct_hash'
> and the 'g_direct_equal' in the g_hash_table_new call.  New routines must be
> written
> to do the key hashing and comparison.

I suspect it's unlikely that you'll find GLib or GTK+ (or perhaps even
Ethereal) working on a platform where a "gint" isn't at least as big as
a "guint32" - not that we're likely to care much about ports to PDP-11s,
Z8000-based systems, or those older 68K-based or 8086/8088/80286-based
systems with 16-bit "int"s.

There could conceivably be a platform where it's bigger, but, in that
case, the compiler should cheerfully convert up.

However, it's probably better to use "GUINT_TO_POINTER()" rather than
"GINT_TO_POINTER()", as the pattern is an unsigned integer.

It appears you compared a tree presumably checked out from CVS with an
older tree that you'd modified, so the patch reverted some changes
checked into CVS.

I've attached a patch against the current CVS tree - I applied only the
parts of your patch that were your changes, as opposed to the
reversions.

I also changed "register_dissector_table()" to return the
"dissector_table_t" for the table - the routine that will be using the
table can just keep a private copy - and modified "dissector_lookup()"
to take that "dissector_table_t" as an argument, which avoids a lookup.

I also changed "dissector_lookup()" to take only one pattern as an
argument - the dissector that uses it to find the next dissector can try
the source port and destination port, or whatever, in order by itself
(which also lets it know *which* port matched, so it could set
"match_port" to let the dissector it called know whether the packet is a
request or reply, if that can't be determined from the packet itself).

I then converted "packet-udp.c" to use it (which is also in the attached
patch); it seemed to work, at least with TFTP.

? errs
Index: packet-udp.c
===================================================================
RCS file: /usr/local/cvsroot/ethereal/packet-udp.c,v
retrieving revision 1.52
diff -c -r1.52 packet-udp.c
*** packet-udp.c	2000/03/12 04:47:50	1.52
--- packet-udp.c	2000/03/18 08:21:40
***************
*** 129,232 ****
  #define UDP_PORT_DHIS1	58800
  #define UDP_PORT_DHIS2	58801
  
! struct hash_struct {
!   guint16 proto;
!   void (*dissect)(const u_char *, int, frame_data *, proto_tree *);
!   struct hash_struct *next;
! };
! 
! static struct hash_struct *hash_table[256];
! 
! /*
!  * These routines are for UDP, will be generalized soon: RJS
!  *
!  * XXX - note that they should probably check the IP address as well as
!  * the port number, so that we don't mistakenly identify packets as, say,
!  * TFTP, merely because they have a source or destination port number
!  * equal to the port being used by a TFTP daemon on some machine other
!  * than the one they're going to or from.
!  */
! 
! struct hash_struct *udp_find_hash_ent(guint16 proto) {
  
-   int idx = proto % 256;
-   struct hash_struct *hash_ent = hash_table[idx];
- 
-   while (hash_ent != NULL) {
- 
-     if (hash_ent -> proto == proto)
-       return hash_ent;
-   
-     hash_ent = hash_ent -> next;
- 
-   }
- 
-   return NULL;
- 
- }
- 
- void udp_hash_add(guint16 proto,
- 	void (*dissect)(const u_char *, int, frame_data *, proto_tree *)) {
- 
-   int idx = proto % 256;   /* Simply take the remainder, hope for no collisions */
-   struct hash_struct *hash_ent = (struct hash_struct *)malloc(sizeof(struct hash_struct));
-   struct hash_struct *hash_ent2;
-   
-   hash_ent -> proto = proto;
-   hash_ent -> dissect = dissect;
-   hash_ent -> next = NULL;
- 
-   if (hash_ent == NULL) {
- 
-     fprintf(stderr, "Could not allocate space for hash structure in dissect_udp\n");
-     exit(1);
-   }
- 
-   if (hash_table[idx]) {  /* Something, add on end */
- 
-     hash_ent2 = hash_table[idx];
- 
-     while (hash_ent2 -> next != NULL)
-       hash_ent2 = hash_ent2 -> next;
- 
-     hash_ent2 -> next = hash_ent;     /* Bad in pathalogical cases */
- 
-   }
-   else {
- 
-     hash_table[idx] = hash_ent;
- 
-   }
- 
- }
- 
  void init_dissect_udp(void) {
- 
-   int i;
  
!   for (i = 0; i < 256; i++) {
! 
!     hash_table[i] = NULL;
! 
!   }
  
    /* Now add the protocols we know about */
  
!   udp_hash_add(UDP_PORT_BOOTPS, dissect_bootp);
!   udp_hash_add(UDP_PORT_TFTP, dissect_tftp);
!   udp_hash_add(UDP_PORT_SAP, dissect_sap);
!   udp_hash_add(UDP_PORT_HSRP, dissect_hsrp);
!   udp_hash_add(UDP_PORT_PIM_RP_DISC, dissect_auto_rp);
!   udp_hash_add(UDP_PORT_TACACS, dissect_tacacs);
!   udp_hash_add(UDP_PORT_DHIS1, dissect_dhis);
!   udp_hash_add(UDP_PORT_DHIS2, dissect_dhis);
  }
  
  void
  dissect_udp(const u_char *pd, int offset, frame_data *fd, proto_tree *tree) {
    e_udphdr  uh;
    guint16    uh_sport, uh_dport, uh_ulen, uh_sum;
!   struct hash_struct *dissect_routine = NULL;
    proto_tree *udp_tree;
    proto_item *ti;
  
--- 129,157 ----
  #define UDP_PORT_DHIS1	58800
  #define UDP_PORT_DHIS2	58801
  
! static dissector_table_t udp_dissector_table;
  
  void init_dissect_udp(void) {
  
!   udp_dissector_table = register_dissector_table(proto_udp);
  
    /* Now add the protocols we know about */
  
!   dissector_add("udp", UDP_PORT_BOOTPS, dissect_bootp);
!   dissector_add("udp", UDP_PORT_TFTP, dissect_tftp);
!   dissector_add("udp", UDP_PORT_SAP, dissect_sap);
!   dissector_add("udp", UDP_PORT_HSRP, dissect_hsrp);
!   dissector_add("udp", UDP_PORT_PIM_RP_DISC, dissect_auto_rp);
!   dissector_add("udp", UDP_PORT_TACACS, dissect_tacacs);
!   dissector_add("udp", UDP_PORT_DHIS1, dissect_dhis);
!   dissector_add("udp", UDP_PORT_DHIS2, dissect_dhis);
  }
  
  void
  dissect_udp(const u_char *pd, int offset, frame_data *fd, proto_tree *tree) {
    e_udphdr  uh;
    guint16    uh_sport, uh_dport, uh_ulen, uh_sum;
!   dissector_t dissect_routine;
    proto_tree *udp_tree;
    proto_item *ti;
  
***************
*** 334,340 ****
        dissect_vines_frp(pd, offset, fd, tree);
    } else if (PORT_IS(UDP_PORT_TFTP)) {
        /* This is the first point of call, but it adds a dynamic call */
!       udp_hash_add(MAX(uh_sport, uh_dport), dissect_tftp);  /* Add to table */
        dissect_tftp(pd, offset, fd, tree);
    } else if (PORT_IS(UDP_PORT_TIME)) {
        dissect_time(pd, offset, fd, tree);
--- 259,265 ----
        dissect_vines_frp(pd, offset, fd, tree);
    } else if (PORT_IS(UDP_PORT_TFTP)) {
        /* This is the first point of call, but it adds a dynamic call */
!       dissector_add("udp", MAX(uh_sport, uh_dport), dissect_tftp);  /* Add to table */
        dissect_tftp(pd, offset, fd, tree);
    } else if (PORT_IS(UDP_PORT_TIME)) {
        dissect_time(pd, offset, fd, tree);
***************
*** 354,374 ****
   } else {
        /* OK, find a routine in the table, else use the default */
  
!       if ((dissect_routine = udp_find_hash_ent(uh_sport))) {
  
! 	struct hash_struct *dr2 = udp_find_hash_ent(uh_dport);
  
  	if (dr2 == NULL) {  /* Not in the table, add */
  
! 	  udp_hash_add(uh_dport, dissect_tftp);
  
  	}
  
! 	dissect_routine -> dissect(pd, offset, fd, tree);
        }
!       else if ((dissect_routine = udp_find_hash_ent(uh_dport))) {
  
! 	dissect_routine -> dissect(pd, offset, fd, tree);
  
        }
        else {
--- 279,299 ----
   } else {
        /* OK, find a routine in the table, else use the default */
  
!       if ((dissect_routine = dissector_lookup(udp_dissector_table, uh_sport))) {
  
! 	dissector_t dr2 = dissector_lookup(udp_dissector_table, uh_dport);
  
  	if (dr2 == NULL) {  /* Not in the table, add */
  
! 	  dissector_add("udp", uh_dport, dissect_tftp);
  
  	}
  
! 	(*dissect_routine)(pd, offset, fd, tree);
        }
!       else if ((dissect_routine = dissector_lookup(udp_dissector_table, uh_dport))) {
  
! 	(*dissect_routine)(pd, offset, fd, tree);
  
        }
        else {
Index: packet.c
===================================================================
RCS file: /usr/local/cvsroot/ethereal/packet.c,v
retrieving revision 1.65
diff -c -r1.65 packet.c
*** packet.c	2000/03/12 04:47:53	1.65
--- packet.c	2000/03/18 08:21:42
***************
*** 1187,1189 ****
--- 1187,1265 ----
  	proto_register_field_array(proto_frame, hf, array_length(hf));
  	proto_register_subtree_array(ett, array_length(ett));
  }
+ 
+ /*********************** code added for sub-dissector lookup *********************/
+ 
+ header_field_info *find_proto_hfinfo( char *abbrev) {
+ 
+ /* get the hfinfo field for the protocol with this abbreviation */
+ 
+ 	int proto_id = find_proto_id( abbrev);
+ 	
+ 	g_assert( proto_id != -1);
+ 
+ 	return find_hfinfo_record(proto_id);
+ }
+ 
+ 
+ dissector_t dissector_lookup( dissector_table_t table, guint32 pattern) {
+ 
+ /* lookup a dissector based upon pattern. */
+ 
+ 	return g_hash_table_lookup( table, GUINT_TO_POINTER( pattern));
+ }
+ 
+ 
+ void dissector_add( char *abbrev, guint32 pattern, dissector_t dissector) {
+ 
+ /* add an entry, lookup the dissector table for the protocol abbreviation,  */
+ /* if a valid table found, add the subdissector */
+ 
+ 	header_field_info *hfinfo = find_proto_hfinfo( abbrev);
+ 
+ /* sanity check */
+ 	g_assert( hfinfo && hfinfo->sub_dissectors);
+ 
+ /* do the table insertion */
+     	g_hash_table_insert( hfinfo->sub_dissectors, GUINT_TO_POINTER( pattern),
+     	 (gpointer)dissector);
+ }
+ 
+ 
+ void dissector_delete( char *abbrev, guint32 pattern, dissector_t dissector) {
+ 
+ /* delete the entry for this dissector at this pattern */
+ 
+ /* NOTE: this doesn't use the dissector call variable. It is included to */
+ /*	be consistant with the dissector_add and more importantly to be used */
+ /*	if the technique of adding a temporary dissector is implimented.  */
+ /*	If temporary dissectors are deleted, then the original dissector must	*/
+ /*	be available. */
+ 
+ 	header_field_info *hfinfo = find_proto_hfinfo( abbrev);
+ 
+ /* sanity check */
+ 	g_assert( hfinfo && hfinfo->sub_dissectors);
+ 
+ /* remove the hash table entry */
+ 	g_hash_table_remove(  hfinfo->sub_dissectors, GUINT_TO_POINTER( pattern));
+ }
+ 
+ 
+ dissector_table_t register_dissector_table( int proto_id){
+ 
+ /* Create and register the dissector array for this protocol; returns */
+ /* a pointer to the dissector table. */
+ 
+ /* NOTE: currently use the g_direct_XXX functions so all the hashing is done  */
+ /* 	by glib and we don't have to create hashing or comparision funtcions. */
+ 	
+ 
+ 	header_field_info *hfinfo = find_hfinfo_record(proto_id);
+ 
+ 	g_assert( hfinfo);	
+ 		
+ 	hfinfo->sub_dissectors = g_hash_table_new( g_direct_hash, g_direct_equal);
+ 	return hfinfo->sub_dissectors;
+ }
+ 
Index: packet.h
===================================================================
RCS file: /usr/local/cvsroot/ethereal/packet.h,v
retrieving revision 1.174
diff -c -r1.174 packet.h
*** packet.h	2000/03/08 06:47:51	1.174
--- packet.h	2000/03/18 08:21:43
***************
*** 205,210 ****
--- 205,228 ----
  } true_false_string;
  
  
+ /* types for sub-dissector lookup */
+ typedef void (*dissector_t)(const u_char *, int, frame_data *, proto_tree *);
+ 
+ /* a protocol uses the function to register its sub-dissector table */
+ dissector_table_t register_dissector_table( int proto_id);
+ 
+ /* dissector lookup routine.  called by protocol dissector to find a sub-dissector */
+ dissector_t dissector_lookup( dissector_table_t table, guint32 pattern);
+ 
+ /* Add a sub-dissector to a dissector table.  Called by the protocol routine */
+ /* that wants to register a sub-dissector.  */
+ void dissector_add( char *abbrev, guint32 pattern, dissector_t dissector);
+ 
+ /* Add a sub-dissector to a dissector table.  Called by the protocol routine */
+ /* that wants to de-register a sub-dissector.  */
+ void dissector_delete( char *abbrev, guint32 pattern, dissector_t dissector);
+ 
+ 
  /* Many of the structs and definitions below and in packet-*.c files
   * were taken from include files in the Linux distribution. */
  
***************
*** 267,274 ****
  
  void init_dissect_rpc(void);
  void init_dissect_udp(void);
- 
- typedef void	(*DissectFunc)	(const u_char*, int, frame_data*, proto_tree*);
  
  /*
   * Routines should take four args: packet data *, offset, frame_data *,
--- 285,290 ----
Index: proto.c
===================================================================
RCS file: /usr/local/cvsroot/ethereal/proto.c,v
retrieving revision 1.56
diff -c -r1.56 proto.c
*** proto.c	2000/03/14 06:03:25	1.56
--- proto.c	2000/03/18 08:21:46
***************
*** 72,81 ****
  static gboolean
  proto_tree_free_node(GNode *node, gpointer data);
  
- static struct header_field_info*
- find_hfinfo_record(int hfindex);
- 
- 
  static void fill_label_boolean(field_info *fi, gchar *label_str);
  static void fill_label_uint(field_info *fi, gchar *label_str);
  static void fill_label_enumerated_uint(field_info *fi, gchar *label_str);
--- 72,77 ----
***************
*** 224,229 ****
--- 220,250 ----
  		g_ptr_array_free(gpa_hfinfo, FALSE);
  }
  
+ /******************** code added for sub-dissector lookup *********************/
+ 
+ int find_proto_id(char *abbrev){
+ 
+ /* find the protocol id based upon the protocol abbreviation */
+ /* return -1 if it fails */
+ 
+ 	header_field_info	*hfinfo;
+ 	int			i, len;
+ 
+ 	len = gpa_hfinfo->len;
+ 	for (i = 0; i < len ; i++) {
+ 		hfinfo = find_hfinfo_record(i);
+ 
+ 		/* format for protocols */
+ 		if (proto_registrar_is_protocol(i)) {
+ 			if ( !strcmp(abbrev, hfinfo->abbrev))
+ 				return hfinfo->id;
+ 		}
+ 	}
+ 	return -1;
+ }
+ 
+ 
+ 
  /* frees the resources that the dissection a proto_tree uses */
  void
  proto_tree_free(proto_tree *tree)
***************
*** 251,257 ****
  }	
  
  /* Finds a record in the hf_info_records array. */
! static struct header_field_info*
  find_hfinfo_record(int hfindex)
  {
  	g_assert(hfindex >= 0 && hfindex < gpa_hfinfo->len);
--- 272,278 ----
  }	
  
  /* Finds a record in the hf_info_records array. */
! struct header_field_info*
  find_hfinfo_record(int hfindex)
  {
  	g_assert(hfindex >= 0 && hfindex < gpa_hfinfo->len);
***************
*** 929,934 ****
--- 950,957 ----
  	hfinfo->bitshift = 0;
  	hfinfo->blurb = "";
  	hfinfo->parent = -1; /* this field differentiates protos and fields */
+ 
+ 	hfinfo->sub_dissectors = NULL;	/* clear sub-dissector table pointer */
  
  	return proto_register_field_init(hfinfo, hfinfo->parent);
  }
Index: proto.h
===================================================================
RCS file: /usr/local/cvsroot/ethereal/proto.h,v
retrieving revision 1.25
diff -c -r1.25 proto.h
*** proto.h	2000/03/14 06:03:26	1.25
--- proto.h	2000/03/18 08:21:47
***************
*** 89,94 ****
--- 89,98 ----
  	BASE_BIN
  };
  
+ 
+ /* types for sub-dissector lookup */
+ typedef GHashTable* dissector_table_t;
+ 
  /* information describing a header field */
  typedef struct header_field_info {
  	char				*name;
***************
*** 102,109 ****
--- 106,116 ----
  	int				id;		/* assigned by registration function, not programmer */
  	int				parent;		/* parent protocol */
  	int				bitshift;	/* bits to shift */
+ 	dissector_table_t 		sub_dissectors; /* sub-dissector table pointer */
  } header_field_info;
  
+ 
+ 
  /* Used when registering many fields at once */
  typedef struct hf_register_info {
  	int			*p_id;	/* pointer to int; written to by register() function */
***************
*** 331,336 ****
--- 338,348 ----
  
  /* Returns char* to abbrev for item # n (0-indexed) */
  char* proto_registrar_get_abbrev(int n);
+ 
+ /* get the protocol information based upon its id */
+ struct header_field_info* find_hfinfo_record(int hfindex);
+ 
+ int find_proto_id(char *abbrev);
  
  /* Returns enum ftenum for item # n */
  int proto_registrar_get_ftype(int n);