Work on patch #9044: SNMP response for failed get operation

Inspired by Marco Veeneman. Change signature of get_value function to return s16_t, where values <0 indicate an error. This is mapped to SNMP_ERR_GENERROR.

src/apps/snmp/snmp_mib2_icmp.c

@@ -55,7 +55,7 @@
 
 /* --- icmp .1.3.6.1.2.1.5 ----------------------------------------------------- */
 
-static u16_t
+static s16_t
 icmp_get_value(const struct snmp_scalar_array_node_def *node, void *value)
 {
   u32_t *uint_ptr = (u32_t*)value;

src/apps/snmp/snmp_mib2_interfaces.c

@@ -58,7 +58,7 @@
 
 /* --- interfaces .1.3.6.1.2.1.2 ----------------------------------------------------- */
 
-static u16_t 
+static s16_t 
 interfaces_get_value(struct snmp_node_instance* instance, void* value)
 {
   if (instance->node->oid == 1) {
@@ -159,7 +159,7 @@ interfaces_Table_get_next_cell_instance(const u32_t* column, struct snmp_obj_id*
   return SNMP_ERR_NOSUCHINSTANCE;
 }
 
-static u16_t
+static s16_t
 interfaces_Table_get_value(struct snmp_node_instance* instance, void* value)
 {
   struct netif *netif = (struct netif*)instance->reference.ptr;

src/apps/snmp/snmp_mib2_ip.c

@@ -58,7 +58,7 @@
 #if LWIP_IPV4
 /* --- ip .1.3.6.1.2.1.4 ----------------------------------------------------- */
 
-static u16_t 
+static s16_t 
 ip_get_value(struct snmp_node_instance* instance, void* value)
 {
   s32_t* sint_ptr = (s32_t*)value;

src/apps/snmp/snmp_mib2_snmp.c

@@ -45,7 +45,7 @@
 #define MIB2_AUTH_TRAPS_DISABLED 2
 
 /* --- snmp .1.3.6.1.2.1.11 ----------------------------------------------------- */
-static u16_t
+static s16_t
 snmp_get_value(const struct snmp_scalar_array_node_def *node, void *value)
 {
   u32_t *uint_ptr = (u32_t*)value;

src/apps/snmp/snmp_mib2_system.c

@@ -213,17 +213,17 @@ snmp_mib2_set_syslocation_readonly(const u8_t *ocstr, const u16_t *ocstrlen)
 }
 
 
-static u16_t
+static s16_t
 system_get_value(const struct snmp_scalar_array_node_def *node, void *value)
 {
   const u8_t*  var = NULL;
-  const u16_t* var_len;
+  const s16_t* var_len;
   u16_t result;
 
   switch (node->oid) {
   case 1: /* sysDescr */
     var     = sysdescr;
-    var_len = sysdescr_len;
+    var_len = (s16_t*)sysdescr_len;
     break;
   case 2: /* sysObjectID */
     {
@@ -236,15 +236,15 @@ system_get_value(const struct snmp_scalar_array_node_def *node, void *value)
     return sizeof(u32_t);
   case 4: /* sysContact */
     var     = syscontact;
-    var_len = syscontact_len;
+    var_len = (s16_t*)syscontact_len;
     break;
   case 5: /* sysName */
     var     = sysname;
-    var_len = sysname_len;
+    var_len = (s16_t*)sysname_len;
     break;
   case 6: /* sysLocation */
     var     = syslocation;
-    var_len = syslocation_len;
+    var_len = (s16_t*)syslocation_len;
     break;
   case 7: /* sysServices */
     *(s32_t*)value = SNMP_SYSSERVICES;
@@ -257,7 +257,7 @@ system_get_value(const struct snmp_scalar_array_node_def *node, void *value)
   /* handle string values (OID 1,4,5 and 6) */
   LWIP_ASSERT("", (value != NULL));
   if (var_len == NULL) {
-    result = (u16_t)strlen((const char*)var);
+    result = (s16_t)strlen((const char*)var);
   } else {
     result = *var_len;
   }

src/apps/snmp/snmp_mib2_tcp.c

@@ -58,7 +58,7 @@
 
 /* --- tcp .1.3.6.1.2.1.6 ----------------------------------------------------- */
 
-static u16_t 
+static s16_t 
 tcp_get_value(struct snmp_node_instance* instance, void* value)
 {
   u32_t *uint_ptr = (u32_t*)value;

src/apps/snmp/snmp_mib2_udp.c

@@ -57,7 +57,7 @@
 
 /* --- udp .1.3.6.1.2.1.7 ----------------------------------------------------- */
 
-static u16_t 
+static s16_t 
 udp_get_value(struct snmp_node_instance* instance, void* value)
 {
   u32_t *uint_ptr = (u32_t*)value;

src/apps/snmp/snmp_msg.c

@@ -285,14 +285,21 @@ snmp_process_varbind(struct snmp_request *request, struct snmp_varbind *vb, u8_t
       request->error_status = SNMP_ERR_GENERROR;
     }
   } else {
+    s16_t len = node_instance.get_value(&node_instance, vb->value);
     vb->type = node_instance.asn1_type;
-    vb->value_len = node_instance.get_value(&node_instance, vb->value);
-    LWIP_ASSERT("SNMP_MAX_VALUE_SIZE is configured too low", (vb->value_len & ~SNMP_GET_VALUE_RAW_DATA) <= SNMP_MAX_VALUE_SIZE);
 
-    err = snmp_append_outbound_varbind(&(request->outbound_pbuf_stream), vb);
+    if(len >= 0) {
-    if (err == ERR_BUF) {
+      vb->value_len = (u16_t)len; /* cast is OK because we checked >= 0 above */
-      request->error_status = SNMP_ERR_TOOBIG;
+
-    } else if (err != ERR_OK) {
+      LWIP_ASSERT("SNMP_MAX_VALUE_SIZE is configured too low", (vb->value_len & ~SNMP_GET_VALUE_RAW_DATA) <= SNMP_MAX_VALUE_SIZE);
+      err = snmp_append_outbound_varbind(&request->outbound_pbuf_stream, vb);
+
+      if (err == ERR_BUF) {
+        request->error_status = SNMP_ERR_TOOBIG;
+      } else if (err != ERR_OK) {
+        request->error_status = SNMP_ERR_GENERROR;
+      }
+    } else {
       request->error_status = SNMP_ERR_GENERROR;
     }
 

src/apps/snmp/snmp_scalar.c

@@ -42,7 +42,7 @@
 #include "lwip/apps/snmp_scalar.h"
 #include "lwip/apps/snmp_core.h"
 
-static u16_t snmp_scalar_array_get_value(struct snmp_node_instance* instance, void* value);
+static s16_t snmp_scalar_array_get_value(struct snmp_node_instance* instance, void* value);
 static snmp_err_t  snmp_scalar_array_set_test(struct snmp_node_instance* instance, u16_t value_len, void* value);
 static snmp_err_t  snmp_scalar_array_set_value(struct snmp_node_instance* instance, u16_t value_len, void* value);
 
@@ -190,7 +190,7 @@ snmp_scalar_array_get_next_instance(const u32_t *root_oid, u8_t root_oid_len, st
   return SNMP_ERR_NOERROR;
 }
 
-static u16_t
+static s16_t
 snmp_scalar_array_get_value(struct snmp_node_instance* instance, void* value)
 {
   const struct snmp_scalar_array_node* array_node = (const struct snmp_scalar_array_node*)(const void*)instance->node;

src/apps/snmp/snmp_table.c

@@ -317,7 +317,7 @@ snmp_err_t snmp_table_simple_get_next_instance(const u32_t *root_oid, u8_t root_
 }
 
 
-u16_t
+s16_t
 snmp_table_extract_value_from_s32ref(struct snmp_node_instance* instance, void* value)
 {
   s32_t *dst = (s32_t*)value;
@@ -325,7 +325,7 @@ snmp_table_extract_value_from_s32ref(struct snmp_node_instance* instance, void*
   return sizeof(*dst);
 }
 
-u16_t
+s16_t
 snmp_table_extract_value_from_u32ref(struct snmp_node_instance* instance, void* value)
 {
   u32_t *dst = (u32_t*)value;
@@ -333,7 +333,7 @@ snmp_table_extract_value_from_u32ref(struct snmp_node_instance* instance, void*
   return sizeof(*dst);
 }
 
-u16_t
+s16_t
 snmp_table_extract_value_from_refconstptr(struct snmp_node_instance* instance, void* value)
 {
   MEMCPY(value, instance->reference.const_ptr, instance->reference_len);

src/apps/snmp/snmp_threadsync.c

@@ -55,12 +55,12 @@ threadsync_get_value_synced(void *ctx)
 {
   struct threadsync_data *call_data = (struct threadsync_data*)ctx;
 
-  call_data->retval.u16 = call_data->proxy_instance.get_value(&call_data->proxy_instance, call_data->arg1.value);
+  call_data->retval.s16 = call_data->proxy_instance.get_value(&call_data->proxy_instance, call_data->arg1.value);
 
   sys_sem_signal(&call_data->threadsync_node->instance->sem);
 }
 
-static u16_t
+static s16_t
 threadsync_get_value(struct snmp_node_instance* instance, void* value)
 {
   struct threadsync_data *call_data = (struct threadsync_data*)instance->reference.ptr;
@@ -68,7 +68,7 @@ threadsync_get_value(struct snmp_node_instance* instance, void* value)
   call_data->arg1.value = value;
   call_synced_function(call_data, threadsync_get_value_synced);
 
-  return call_data->retval.u16;
+  return call_data->retval.s16;
 }
 
 static void

src/include/lwip/apps/snmp_core.h

@@ -181,7 +181,7 @@ typedef enum {
 
 struct snmp_node_instance;
 
-typedef u16_t (*node_instance_get_value_method)(struct snmp_node_instance*, void*);
+typedef s16_t (*node_instance_get_value_method)(struct snmp_node_instance*, void*);
 typedef snmp_err_t (*node_instance_set_test_method)(struct snmp_node_instance*, u16_t, void*);
 typedef snmp_err_t (*node_instance_set_value_method)(struct snmp_node_instance*, u16_t, void*);
 typedef void (*node_instance_release_method)(struct snmp_node_instance*);
@@ -201,7 +201,7 @@ struct snmp_node_instance
   /** one out of instance access types defined above (SNMP_NODE_INSTANCE_READ_ONLY,...) */
   snmp_access_t access;
   
-  /** returns object value for the given object identifier */
+  /** returns object value for the given object identifier. Return values <0 to indicate an error */
   node_instance_get_value_method get_value;
   /** tests length and/or range BEFORE setting */
   node_instance_set_test_method set_test;

src/include/lwip/apps/snmp_scalar.h

@@ -79,7 +79,7 @@ struct snmp_scalar_array_node_def
   snmp_access_t access;
 };
 
-typedef u16_t (*snmp_scalar_array_get_value_method)(const struct snmp_scalar_array_node_def*, void*);
+typedef s16_t (*snmp_scalar_array_get_value_method)(const struct snmp_scalar_array_node_def*, void*);
 typedef snmp_err_t (*snmp_scalar_array_set_test_method)(const struct snmp_scalar_array_node_def*, u16_t, void*);
 typedef snmp_err_t (*snmp_scalar_array_set_value_method)(const struct snmp_scalar_array_node_def*, u16_t, void*);
 

src/include/lwip/apps/snmp_table.h

@@ -121,9 +121,9 @@ snmp_err_t snmp_table_simple_get_next_instance(const u32_t *root_oid, u8_t root_
   snmp_table_simple_get_next_instance }, \
   (u16_t)LWIP_ARRAYSIZE(columns), (columns), (get_cell_value_method), (get_next_cell_instance_and_value_method) }
 
-u16_t snmp_table_extract_value_from_s32ref(struct snmp_node_instance* instance, void* value);
+s16_t snmp_table_extract_value_from_s32ref(struct snmp_node_instance* instance, void* value);
-u16_t snmp_table_extract_value_from_u32ref(struct snmp_node_instance* instance, void* value);
+s16_t snmp_table_extract_value_from_u32ref(struct snmp_node_instance* instance, void* value);
-u16_t snmp_table_extract_value_from_refconstptr(struct snmp_node_instance* instance, void* value);
+s16_t snmp_table_extract_value_from_refconstptr(struct snmp_node_instance* instance, void* value);
 
 #endif /* LWIP_SNMP */
 

src/include/lwip/apps/snmp_threadsync.h

@@ -58,7 +58,7 @@ struct threadsync_data
 {
   union {
     snmp_err_t u8;
-    u16_t u16;
+    s16_t s16;
   } retval;
   union {
     const u32_t *root_oid;