Merge pull request #8846 from ton31337/fix/some_code_cleanups

bgpd: A couple of cleanups

bgpd/bgp_aspath.c

@@ -910,77 +910,70 @@ size_t aspath_put(struct stream *s, struct aspath *as, int use32bit)
 	if (!seg || seg->length == 0)
 		return 0;
 
-	if (seg) {
+	/*
-		/*
+	 * Hey, what do we do when we have > STREAM_WRITABLE(s) here?
-		 * Hey, what do we do when we have > STREAM_WRITABLE(s) here?
+	 * At the moment, we would write out a partial aspath, and our
-		 * At the moment, we would write out a partial aspath, and our
+	 * peer
-		 * peer
+	 * will complain and drop the session :-/
-		 * will complain and drop the session :-/
+	 *
-		 *
+	 * The general assumption here is that many things tested will
-		 * The general assumption here is that many things tested will
+	 * never happen.  And, in real live, up to now, they have not.
-		 * never happen.  And, in real live, up to now, they have not.
+	 */
-		 */
+	while (seg && (ASSEGMENT_LEN(seg, use32bit) <= STREAM_WRITEABLE(s))) {
-		while (seg && (ASSEGMENT_LEN(seg, use32bit)
+		struct assegment *next = seg->next;
-			       <= STREAM_WRITEABLE(s))) {
+		int written = 0;
-			struct assegment *next = seg->next;
+		int asns_packed = 0;
-			int written = 0;
+		size_t lenp;
-			int asns_packed = 0;
-			size_t lenp;
 
-			/* Overlength segments have to be split up */
+		/* Overlength segments have to be split up */
-			while ((seg->length - written) > AS_SEGMENT_MAX) {
+		while ((seg->length - written) > AS_SEGMENT_MAX) {
-				assegment_header_put(s, seg->type,
+			assegment_header_put(s, seg->type, AS_SEGMENT_MAX);
-						     AS_SEGMENT_MAX);
-				assegment_data_put(s, (seg->as + written), AS_SEGMENT_MAX,
-						   use32bit);
-				written += AS_SEGMENT_MAX;
-				bytes += ASSEGMENT_SIZE(AS_SEGMENT_MAX,
-							use32bit);
-			}
-
-			/* write the final segment, probably is also the first
-			 */
-			lenp = assegment_header_put(s, seg->type,
-						    seg->length - written);
 			assegment_data_put(s, (seg->as + written),
-					   seg->length - written, use32bit);
+					   AS_SEGMENT_MAX, use32bit);
-
+			written += AS_SEGMENT_MAX;
-			/* Sequence-type segments can be 'packed' together
+			bytes += ASSEGMENT_SIZE(AS_SEGMENT_MAX, use32bit);
-			 * Case of a segment which was overlength and split up
-			 * will be missed here, but that doesn't matter.
-			 */
-			while (next && ASSEGMENTS_PACKABLE(seg, next)) {
-				/* NB: We should never normally get here given
-				 * we
-				 * normalise aspath data when parse them.
-				 * However, better
-				 * safe than sorry. We potentially could call
-				 * assegment_normalise here instead, but it's
-				 * cheaper and
-				 * easier to do it on the fly here rather than
-				 * go through
-				 * the segment list twice every time we write
-				 * out
-				 * aspath's.
-				 */
-
-				/* Next segment's data can fit in this one */
-				assegment_data_put(s, next->as, next->length,
-						   use32bit);
-
-				/* update the length of the segment header */
-				stream_putc_at(s, lenp,
-					       seg->length - written
-						       + next->length);
-				asns_packed += next->length;
-
-				next = next->next;
-			}
-
-			bytes += ASSEGMENT_SIZE(
-				seg->length - written + asns_packed, use32bit);
-			seg = next;
 		}
+
+		/* write the final segment, probably is also the first
+		 */
+		lenp = assegment_header_put(s, seg->type,
+					    seg->length - written);
+		assegment_data_put(s, (seg->as + written),
+				   seg->length - written, use32bit);
+
+		/* Sequence-type segments can be 'packed' together
+		 * Case of a segment which was overlength and split up
+		 * will be missed here, but that doesn't matter.
+		 */
+		while (next && ASSEGMENTS_PACKABLE(seg, next)) {
+			/* NB: We should never normally get here given
+			 * we
+			 * normalise aspath data when parse them.
+			 * However, better
+			 * safe than sorry. We potentially could call
+			 * assegment_normalise here instead, but it's
+			 * cheaper and
+			 * easier to do it on the fly here rather than
+			 * go through
+			 * the segment list twice every time we write
+			 * out
+			 * aspath's.
+			 */
+
+			/* Next segment's data can fit in this one */
+			assegment_data_put(s, next->as, next->length, use32bit);
+
+			/* update the length of the segment header */
+			stream_putc_at(s, lenp,
+				       seg->length - written + next->length);
+			asns_packed += next->length;
+
+			next = next->next;
+		}
+
+		bytes += ASSEGMENT_SIZE(seg->length - written + asns_packed,
+					use32bit);
+		seg = next;
 	}
 	return bytes;
 }

bgpd/bgp_damp.c

@@ -245,7 +245,6 @@ static int bgp_reuse_timer(struct thread *t)
 	 * list head entry. */
 	assert(bdc->reuse_offset < bdc->reuse_list_size);
 	plist = bdc->reuse_list[bdc->reuse_offset];
-	node = SLIST_FIRST(&plist);
 	SLIST_INIT(&bdc->reuse_list[bdc->reuse_offset]);
 
 	/* 2.  set offset = modulo reuse-list-size ( offset + 1 ), thereby
@@ -788,7 +787,7 @@ const char *bgp_damp_reuse_time_vty(struct vty *vty, struct bgp_path_info *path,
 
 	/* If dampening is not enabled or there is no dampening information,
 	   return immediately.  */
-	if (!bdc || !bdi)
+	if (!bdi)
 		return NULL;
 
 	/* Calculate new penalty.  */

bgpd/bgp_evpn_mh.c

@@ -2070,9 +2070,8 @@ int bgp_evpn_local_es_del(struct bgp *bgp, esi_t *esi)
 	/* Lookup ESI hash - should exist. */
 	es = bgp_evpn_es_find(esi);
 	if (!es) {
-		flog_warn(EC_BGP_EVPN_ESI,
+		flog_warn(EC_BGP_EVPN_ESI, "%u: ES missing at local ES DEL",
-			  "%u: ES %s missing at local ES DEL",
+			  bgp->vrf_id);
-			  bgp->vrf_id, es->esi_str);
 		return -1;
 	}
 
@@ -3317,9 +3316,6 @@ bgp_evpn_es_evi_local_info_clear(struct bgp_evpn_es_evi *es_evi)
 {
 	struct bgpevpn *vpn = es_evi->vpn;
 
-	if (!CHECK_FLAG(es_evi->flags, BGP_EVPNES_EVI_LOCAL))
-		return es_evi;
-
 	UNSET_FLAG(es_evi->flags, BGP_EVPNES_EVI_LOCAL);
 	list_delete_node(vpn->local_es_evi_list, &es_evi->l2vni_listnode);
 

bgpd/bgp_evpn_vty.c

@@ -768,13 +768,13 @@ static void bgp_evpn_show_routes_mac_ip_es(struct vty *vty, esi_t *esi,
 static void bgp_evpn_show_routes_mac_ip_evi_es(struct vty *vty, esi_t *esi,
 					       json_object *json, int detail)
 {
-	return bgp_evpn_show_routes_mac_ip_es(vty, esi, json, detail, false);
+	bgp_evpn_show_routes_mac_ip_es(vty, esi, json, detail, false);
 }
 
 static void bgp_evpn_show_routes_mac_ip_global_es(struct vty *vty, esi_t *esi,
 						  json_object *json, int detail)
 {
-	return bgp_evpn_show_routes_mac_ip_es(vty, esi, json, detail, true);
+	bgp_evpn_show_routes_mac_ip_es(vty, esi, json, detail, true);
 }
 
 static void show_vni_routes(struct bgp *bgp, struct bgpevpn *vpn, int type,

bgpd/bgp_flowspec_util.c

@@ -641,13 +641,12 @@ int bgp_flowspec_match_rules_fill(uint8_t *nlri_content, int len,
 				 __func__, type);
 		}
 	}
-	if (bpem->match_packet_length_num || bpem->match_fragment_num ||
+	if (bpem->match_packet_length_num || bpem->match_fragment_num
-	    bpem->match_tcpflags_num || bpem->match_dscp_num ||
+	    || bpem->match_tcpflags_num || bpem->match_dscp_num
-	    bpem->match_packet_length_num || bpem->match_icmp_code_num ||
+	    || bpem->match_icmp_code_num || bpem->match_icmp_type_num
-	    bpem->match_icmp_type_num || bpem->match_port_num ||
+	    || bpem->match_port_num || bpem->match_src_port_num
-	    bpem->match_src_port_num || bpem->match_dst_port_num ||
+	    || bpem->match_dst_port_num || bpem->match_protocol_num
-	    bpem->match_protocol_num || bpem->match_bitmask ||
+	    || bpem->match_bitmask || bpem->match_flowlabel_num)
-	    bpem->match_flowlabel_num)
 		bpem->type = BGP_PBR_IPSET;
 	else if ((bpem->match_bitmask_iprule & PREFIX_SRC_PRESENT) ||
 		 (bpem->match_bitmask_iprule & PREFIX_DST_PRESENT))

bgpd/rfapi/rfapi.c

@@ -2179,7 +2179,7 @@ int rfapi_close(void *handle)
 		vnc_zlog_debug_verbose("%s administrative close rfd=%p",
 				       __func__, rfd);
 
-		if (h && h->rfp_methods.close_cb) {
+		if (h->rfp_methods.close_cb) {
 			vnc_zlog_debug_verbose(
 				"%s calling close callback rfd=%p", __func__,
 				rfd);

bgpd/rfapi/rfapi_import.c

@@ -2592,10 +2592,8 @@ static void rfapiCopyUnEncap2VPN(struct bgp_path_info *encap_bpi,
 		 * instrumentation to debug segfault of 091127
 		 */
 		vnc_zlog_debug_verbose("%s: vpn_bpi=%p", __func__, vpn_bpi);
-		if (vpn_bpi) {
+		vnc_zlog_debug_verbose("%s: vpn_bpi->extra=%p", __func__,
-			vnc_zlog_debug_verbose("%s: vpn_bpi->extra=%p",
+				       vpn_bpi->extra);
-					       __func__, vpn_bpi->extra);
-		}
 
 		vpn_bpi->extra->vnc.import.un_family = AF_INET;
 		vpn_bpi->extra->vnc.import.un.addr4 =

lib/link_state.c

@@ -79,7 +79,6 @@ void ls_node_del(struct ls_node *node)
 		return;
 
 	XFREE(MTYPE_LS_DB, node);
-	node = NULL;
 }
 
 int ls_node_same(struct ls_node *n1, struct ls_node *n2)
@@ -168,7 +167,6 @@ void ls_attributes_del(struct ls_attributes *attr)
 	ls_attributes_srlg_del(attr);
 
 	XFREE(MTYPE_LS_DB, attr);
-	attr = NULL;
 }
 
 int ls_attributes_same(struct ls_attributes *l1, struct ls_attributes *l2)
@@ -221,7 +219,6 @@ void ls_prefix_del(struct ls_prefix *pref)
 		return;
 
 	XFREE(MTYPE_LS_DB, pref);
-	pref = NULL;
 }
 
 int ls_prefix_same(struct ls_prefix *p1, struct ls_prefix *p2)
@@ -839,7 +836,6 @@ void ls_ted_del(struct ls_ted *ted)
 	subnets_fini(&ted->subnets);
 
 	XFREE(MTYPE_LS_DB, ted);
-	ted = NULL;
 }
 
 void ls_ted_del_all(struct ls_ted *ted)