Wireshark · Ethereal-dev: Re: [ethereal-dev] SNMP Bug Report

[Guy Harris <gharris@xxxxxxxxxxxx>]

On Sun, Jun 25, 2000 at 04:22:16PM -0700, Guy Harris wrote:
> It is probably worth dynamically allocating the buffer in the case where
> it's not linked with "-lsnmp"; however, *all* we can do when it's linked
> with "-lsnmp" is to make the fixed-length buffer longer.

Well, we can do some more - we can, at least, try to guess an upper
bound on the size of the string "sprint_value()" will generate, based on
the size of the object and a guess as to how much it'll generate per
item in the object.

Here's a patch that does that (and that makes "packet-snmp.c" slightly
more closely resemble the libsmi version); I've checked it in.

Index: packet-snmp.c
===================================================================
RCS file: /usr/local/cvsroot/ethereal/packet-snmp.c,v
retrieving revision 1.38
diff -c -r1.38 packet-snmp.c
*** packet-snmp.c	2000/06/25 20:55:09	1.38
--- packet-snmp.c	2000/06/26 00:12:52
***************
*** 569,578 ****
  }
  
  #ifdef HAVE_SPRINT_VALUE
! static void
! format_value(gchar *buf, struct variable_list *variable, subid_t *variable_oid,
      guint variable_oid_length, gushort vb_type, guint vb_length)
  {
  	variable->next_variable = NULL;
  	variable->name = variable_oid;
  	variable->name_length = variable_oid_length;
--- 569,617 ----
  }
  
  #ifdef HAVE_SPRINT_VALUE
! static gchar *
! format_var(struct variable_list *variable, subid_t *variable_oid,
      guint variable_oid_length, gushort vb_type, guint vb_length)
  {
+ 	gchar *buf;
+ 
+ 	switch (vb_type) {
+ 
+ 	case SNMP_INTEGER:
+ 	case SNMP_COUNTER:
+ 	case SNMP_GAUGE:
+ 	case SNMP_TIMETICKS:
+ 		/* We don't know how long this will be, but let's guess it
+ 		   fits within 128 characters; that should be enough for an
+ 		   integral value plus some sort of type indication. */
+ 		buf = g_malloc(128);
+ 		break;
+ 
+ 	case SNMP_OCTETSTR:
+ 	case SNMP_IPADDR:
+ 	case SNMP_OPAQUE:
+ 	case SNMP_NSAP:
+ 	case SNMP_BITSTR:
+ 	case SNMP_COUNTER64:
+ 		/* We don't know how long this will be, but let's guess it
+ 		   fits within 128 characters plus 4 characters per octet. */
+ 		buf = g_malloc(128 + 4*vb_length);
+ 		break;
+ 
+ 	case SNMP_OBJECTID:
+ 		/* We don't know how long this will be, but let's guess it
+ 		   fits within 128 characters plus 32 characters per subid
+ 		   (10 digits plus period, or a subid name). */
+ 		buf = g_malloc(1024 + 32*vb_length);
+ 		break;
+ 
+ 	default:
+ 		/* Should not happen. */
+ 		g_assert_not_reached();
+ 		buf = NULL;
+ 		break;
+ 	}
+ 
  	variable->next_variable = NULL;
  	variable->name = variable_oid;
  	variable->name_length = variable_oid_length;
***************
*** 612,617 ****
--- 651,657 ----
  
  	case SNMP_OBJECTID:
  		variable->type = VALTYPE_OBJECTID;
+ 		vb_length *= sizeof (subid_t);	/* XXX - necessary? */
  		break;
  
  	case SNMP_BITSTR:
***************
*** 623,629 ****
--- 663,671 ----
  		break;
  	}
  	variable->val_len = vb_length;
+ 
  	sprint_value(buf, variable_oid, variable_oid_length, variable);
+ 	return buf;
  }
  #endif
  
***************
*** 648,654 ****
  	subid_t *vb_oid;
  	guint vb_oid_length;
  
! 	gchar vb_display_string[MAX_STRING_LEN]; /* TBC */
  
  #ifdef HAVE_SPRINT_VALUE
  	struct variable_list variable;
--- 690,696 ----
  	subid_t *vb_oid;
  	guint vb_oid_length;
  
! 	gchar *vb_display_string;
  
  #ifdef HAVE_SPRINT_VALUE
  	struct variable_list variable;
***************
*** 697,707 ****
  #elif defined(HAVE_SNMP_SNMP_H)
  			variable.val.integer = &vb_integer_value;
  #endif
! 			format_value(vb_display_string, &variable,
  			    variable_oid, variable_oid_length, vb_type,
  			    vb_length);
  			proto_tree_add_text(snmp_tree, NullTVB, offset, length,
  			    "Value: %s", vb_display_string);
  #else
  			proto_tree_add_text(snmp_tree, NullTVB, offset, length,
  			    "Value: %s: %d (%#x)", vb_type_name,
--- 739,750 ----
  #elif defined(HAVE_SNMP_SNMP_H)
  			variable.val.integer = &vb_integer_value;
  #endif
! 			vb_display_string = format_var(&variable,
  			    variable_oid, variable_oid_length, vb_type,
  			    vb_length);
  			proto_tree_add_text(snmp_tree, NullTVB, offset, length,
  			    "Value: %s", vb_display_string);
+ 			g_free(vb_display_string);
  #else
  			proto_tree_add_text(snmp_tree, NullTVB, offset, length,
  			    "Value: %s: %d (%#x)", vb_type_name,
***************
*** 726,736 ****
  #elif defined(HAVE_SNMP_SNMP_H)
  			variable.val.integer = &vb_uinteger_value;
  #endif
! 			format_value(vb_display_string, &variable,
  			    variable_oid, variable_oid_length, vb_type,
  			    vb_length);
  			proto_tree_add_text(snmp_tree, NullTVB, offset, length,
  			    "Value: %s", vb_display_string);
  #else
  			proto_tree_add_text(snmp_tree, NullTVB, offset, length,
  			    "Value: %s: %u (%#x)", vb_type_name,
--- 769,780 ----
  #elif defined(HAVE_SNMP_SNMP_H)
  			variable.val.integer = &vb_uinteger_value;
  #endif
! 			vb_display_string = format_var(&variable,
  			    variable_oid, variable_oid_length, vb_type,
  			    vb_length);
  			proto_tree_add_text(snmp_tree, NullTVB, offset, length,
  			    "Value: %s", vb_display_string);
+ 			g_free(vb_display_string);
  #else
  			proto_tree_add_text(snmp_tree, NullTVB, offset, length,
  			    "Value: %s: %u (%#x)", vb_type_name,
***************
*** 753,763 ****
  		if (snmp_tree) {
  #ifdef HAVE_SPRINT_VALUE
  			variable.val.string = vb_octet_string;
! 			format_value(vb_display_string, &variable,
  			    variable_oid, variable_oid_length, vb_type,
  			    vb_length);
  			proto_tree_add_text(snmp_tree, NullTVB, offset, length,
  			    "Value: %s", vb_display_string);
  #else
  			/*
  			 * If some characters are not printable, display
--- 797,808 ----
  		if (snmp_tree) {
  #ifdef HAVE_SPRINT_VALUE
  			variable.val.string = vb_octet_string;
! 			vb_display_string = format_var(&variable,
  			    variable_oid, variable_oid_length, vb_type,
  			    vb_length);
  			proto_tree_add_text(snmp_tree, NullTVB, offset, length,
  			    "Value: %s", vb_display_string);
+ 			g_free(vb_display_string);
  #else
  			/*
  			 * If some characters are not printable, display
***************
*** 774,779 ****
--- 819,825 ----
  				 * character, before we got to the end
  				 * of the string.
  				 */
+ 				vb_display_string = g_malloc(4*vb_length);
  				buf = &vb_display_string[0];
  				len = sprintf(buf, "%03u", vb_octet_string[0]);
  				buf += len;
***************
*** 785,790 ****
--- 831,837 ----
  				proto_tree_add_text(snmp_tree, NullTVB, offset, length,
  				    "Value: %s: %s", vb_type_name,
  				    vb_display_string);
+ 				g_free(vb_display_string);
  			} else {
  				proto_tree_add_text(snmp_tree, NullTVB, offset, length,
  				    "Value: %s: %.*s", vb_type_name,
***************
*** 815,830 ****
  		if (snmp_tree) {
  #ifdef HAVE_SPRINT_VALUE
  			variable.val.objid = vb_oid;
! 			format_value(vb_display_string, &variable,
  			    variable_oid, variable_oid_length, vb_type,
! 			    vb_length*sizeof (subid_t));
  			proto_tree_add_text(snmp_tree, NullTVB, offset, length,
  			    "Value: %s", vb_display_string);
  #else
  			format_oid(vb_display_string, vb_oid, vb_oid_length);
  			proto_tree_add_text(snmp_tree, NullTVB, offset, length,
  			    "Value: %s: %s", vb_type_name, vb_display_string);
  #endif
  		}
  		g_free(vb_oid);
  		break;
--- 862,881 ----
  		if (snmp_tree) {
  #ifdef HAVE_SPRINT_VALUE
  			variable.val.objid = vb_oid;
! 			vb_display_string = format_var(&variable,
  			    variable_oid, variable_oid_length, vb_type,
! 			    vb_length);
  			proto_tree_add_text(snmp_tree, NullTVB, offset, length,
  			    "Value: %s", vb_display_string);
  #else
+ 			/* This fits with 11 characters per subid (10 digits
+ 			   plus period). */
+ 			vb_display_string = g_malloc(11*vb_oid_length);
  			format_oid(vb_display_string, vb_oid, vb_oid_length);
  			proto_tree_add_text(snmp_tree, NullTVB, offset, length,
  			    "Value: %s: %s", vb_type_name, vb_display_string);
  #endif
+ 			g_free(vb_display_string);
  		}
  		g_free(vb_oid);
  		break;