From 853e33bdb4bc97db1ba1fe1e48e27701147244e8 Mon Sep 17 00:00:00 2001 From: goldsimon Date: Wed, 27 Jan 2010 17:36:37 +0000 Subject: [PATCH] snmp_recv: don't peek the UDP header, p->tot_len does the same; no need for the extra checks at the beginning; don't use so many if/else but if/return instead to make the code more readable --- src/core/snmp/msg_in.c | 172 ++++++++++++++++------------------------- 1 file changed, 67 insertions(+), 105 deletions(-) diff --git a/src/core/snmp/msg_in.c b/src/core/snmp/msg_in.c index ad60f099..a7cedf0a 100644 --- a/src/core/snmp/msg_in.c +++ b/src/core/snmp/msg_in.c @@ -808,121 +808,83 @@ snmp_msg_event(u8_t request_id) static void snmp_recv(void *arg, struct udp_pcb *pcb, struct pbuf *p, struct ip_addr *addr, u16_t port) { - struct udp_hdr *udphdr; + struct snmp_msg_pstat *msg_ps; + u8_t req_idx; + err_t err_ret; + u16_t payload_len = p->tot_len; + u16_t payload_ofs = 0; + u16_t varbind_ofs = 0; /* suppress unused argument warning */ LWIP_UNUSED_ARG(arg); - /* peek in the UDP header (goto IP payload) */ - if(pbuf_header(p, UDP_HLEN)){ - LWIP_ASSERT("Can't move to UDP header", 0); + + /* traverse input message process list, look for SNMP_MSG_EMPTY */ + msg_ps = &msg_input_list[0]; + req_idx = 0; + while ((req_idx < SNMP_CONCURRENT_REQUESTS) && (msg_ps->state != SNMP_MSG_EMPTY)) + { + req_idx++; + msg_ps++; + } + if (req_idx == SNMP_CONCURRENT_REQUESTS) + { + /* exceeding number of concurrent requests */ pbuf_free(p); return; } - udphdr = p->payload; - /* check if datagram is really directed at us (including broadcast requests) */ - if ((pcb == snmp1_pcb) && (ntohs(udphdr->dest) == SNMP_IN_PORT)) + /* accepting request */ + snmp_inc_snmpinpkts(); + /* record used 'protocol control block' */ + msg_ps->pcb = pcb; + /* source address (network order) */ + msg_ps->sip = *addr; + /* source port (host order (lwIP oddity)) */ + msg_ps->sp = port; + + /* check total length, version, community, pdu type */ + err_ret = snmp_pdu_header_check(p, payload_ofs, payload_len, &varbind_ofs, msg_ps); + /* Only accept requests and requests without error (be robust) */ + /* Reject response and trap headers or error requests as input! */ + if ((err_ret != ERR_OK) || + ((msg_ps->rt != SNMP_ASN1_PDU_GET_REQ) && + (msg_ps->rt != SNMP_ASN1_PDU_GET_NEXT_REQ) && + (msg_ps->rt != SNMP_ASN1_PDU_SET_REQ)) || + ((msg_ps->error_status != SNMP_ES_NOERROR) || + (msg_ps->error_index != 0)) ) { - struct snmp_msg_pstat *msg_ps; - u8_t req_idx; - - /* traverse input message process list, look for SNMP_MSG_EMPTY */ - msg_ps = &msg_input_list[0]; - req_idx = 0; - while ((req_idxstate != SNMP_MSG_EMPTY)) - { - req_idx++; - msg_ps++; - } - if (req_idx != SNMP_CONCURRENT_REQUESTS) - { - err_t err_ret; - u16_t payload_len; - u16_t payload_ofs; - u16_t varbind_ofs = 0; - - /* accepting request */ - snmp_inc_snmpinpkts(); - /* record used 'protocol control block' */ - msg_ps->pcb = pcb; - /* source address (network order) */ - msg_ps->sip = *addr; - /* source port (host order (lwIP oddity)) */ - msg_ps->sp = port; - /* read UDP payload length from UDP header */ - payload_len = ntohs(udphdr->len) - UDP_HLEN; - - /* adjust to UDP payload */ - payload_ofs = UDP_HLEN; - - /* check total length, version, community, pdu type */ - err_ret = snmp_pdu_header_check(p, payload_ofs, payload_len, &varbind_ofs, msg_ps); - if (((msg_ps->rt == SNMP_ASN1_PDU_GET_REQ) || - (msg_ps->rt == SNMP_ASN1_PDU_GET_NEXT_REQ) || - (msg_ps->rt == SNMP_ASN1_PDU_SET_REQ)) && - ((msg_ps->error_status == SNMP_ES_NOERROR) && - (msg_ps->error_index == 0)) ) - { - /* Only accept requests and requests without error (be robust) */ - err_ret = err_ret; - } - else - { - /* Reject response and trap headers or error requests as input! */ - err_ret = ERR_ARG; - } - if (err_ret == ERR_OK) - { - LWIP_DEBUGF(SNMP_MSG_DEBUG, ("snmp_recv ok, community %s\n", msg_ps->community)); - - /* Builds a list of variable bindings. Copy the varbinds from the pbuf - chain to glue them when these are divided over two or more pbuf's. */ - err_ret = snmp_pdu_dec_varbindlist(p, varbind_ofs, &varbind_ofs, msg_ps); - if ((err_ret == ERR_OK) && (msg_ps->invb.count > 0)) - { - /* we've decoded the incoming message, release input msg now */ - pbuf_free(p); - - msg_ps->error_status = SNMP_ES_NOERROR; - msg_ps->error_index = 0; - /* find object for each variable binding */ - msg_ps->state = SNMP_MSG_SEARCH_OBJ; - /* first variable binding from list to inspect */ - msg_ps->vb_idx = 0; - - LWIP_DEBUGF(SNMP_MSG_DEBUG, ("snmp_recv varbind cnt=%"U16_F"\n",(u16_t)msg_ps->invb.count)); - - /* handle input event and as much objects as possible in one go */ - snmp_msg_event(req_idx); - } - else - { - /* varbind-list decode failed, or varbind list empty. - drop request silently, do not return error! - (errors are only returned for a specific varbind failure) */ - pbuf_free(p); - LWIP_DEBUGF(SNMP_MSG_DEBUG, ("snmp_pdu_dec_varbindlist() failed\n")); - } - } - else - { - /* header check failed - drop request silently, do not return error! */ - pbuf_free(p); - LWIP_DEBUGF(SNMP_MSG_DEBUG, ("snmp_pdu_header_check() failed\n")); - } - } - else - { - /* exceeding number of concurrent requests */ - pbuf_free(p); - } - } - else - { - /* datagram not for us */ + /* header check failed drop request silently, do not return error! */ pbuf_free(p); + LWIP_DEBUGF(SNMP_MSG_DEBUG, ("snmp_pdu_header_check() failed\n")); + return; } + LWIP_DEBUGF(SNMP_MSG_DEBUG, ("snmp_recv ok, community %s\n", msg_ps->community)); + + /* Builds a list of variable bindings. Copy the varbinds from the pbuf + chain to glue them when these are divided over two or more pbuf's. */ + err_ret = snmp_pdu_dec_varbindlist(p, varbind_ofs, &varbind_ofs, msg_ps); + /* we've decoded the incoming message, release input msg now */ + pbuf_free(p); + if ((err_ret != ERR_OK) || (msg_ps->invb.count == 0)) + { + /* varbind-list decode failed, or varbind list empty. + drop request silently, do not return error! + (errors are only returned for a specific varbind failure) */ + LWIP_DEBUGF(SNMP_MSG_DEBUG, ("snmp_pdu_dec_varbindlist() failed\n")); + return; + } + + msg_ps->error_status = SNMP_ES_NOERROR; + msg_ps->error_index = 0; + /* find object for each variable binding */ + msg_ps->state = SNMP_MSG_SEARCH_OBJ; + /* first variable binding from list to inspect */ + msg_ps->vb_idx = 0; + + LWIP_DEBUGF(SNMP_MSG_DEBUG, ("snmp_recv varbind cnt=%"U16_F"\n",(u16_t)msg_ps->invb.count)); + + /* handle input event and as much objects as possible in one go */ + snmp_msg_event(req_idx); } /**