Merge pull request #8942 from ton31337/fix/cleanups_2

Another round of cleanup

bfdd/dplane.c

@@ -651,8 +651,6 @@ static struct bfd_dplane_ctx *bfd_dplane_ctx_new(int sock)
 	struct bfd_dplane_ctx *bdc;
 
 	bdc = XCALLOC(MTYPE_BFDD_DPLANE_CTX, sizeof(*bdc));
-	if (bdc == NULL)
-		return NULL;
 
 	bdc->sock = sock;
 	bdc->inbuf = stream_new(BFD_DPLANE_CLIENT_BUF_SIZE);

bgpd/bgp_aspath.c

@@ -193,7 +193,8 @@ static struct assegment *assegment_prepend_asns(struct assegment *seg,
 	if (num >= AS_SEGMENT_MAX)
 		return seg; /* we don't do huge prepends */
 
-	if ((newas = assegment_data_new(seg->length + num)) == NULL)
+	newas = assegment_data_new(seg->length + num);
+	if (newas == NULL)
 		return seg;
 
 	for (i = 0; i < num; i++)

bgpd/bgp_attr.c

@@ -3317,7 +3317,8 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
 	}
 
 	/* Check all mandatory well-known attributes are present */
-	if ((ret = bgp_attr_check(peer, attr)) < 0)
+	ret = bgp_attr_check(peer, attr);
+	if (ret < 0)
 		goto done;
 
 	/*

bgpd/bgp_evpn_mh.c

@@ -1311,7 +1311,7 @@ static void bgp_evpn_es_vtep_re_eval_active(struct bgp *bgp,
 	bool old_active;
 	bool new_active;
 
-	old_active = !!CHECK_FLAG(es_vtep->flags, BGP_EVPNES_VTEP_ACTIVE);
+	old_active = CHECK_FLAG(es_vtep->flags, BGP_EVPNES_VTEP_ACTIVE);
 	/* currently we need an active EVI reference to use the VTEP as
 	 * a nexthop. this may change...
 	 */
@@ -1320,7 +1320,7 @@ static void bgp_evpn_es_vtep_re_eval_active(struct bgp *bgp,
 	else
 		UNSET_FLAG(es_vtep->flags, BGP_EVPNES_VTEP_ACTIVE);
 
-	new_active = !!CHECK_FLAG(es_vtep->flags, BGP_EVPNES_VTEP_ACTIVE);
+	new_active = CHECK_FLAG(es_vtep->flags, BGP_EVPNES_VTEP_ACTIVE);
 
 	if ((old_active != new_active) || (new_active && param_change)) {
 
@@ -3119,7 +3119,7 @@ static void bgp_evpn_es_evi_vtep_re_eval_active(struct bgp *bgp,
 	bool new_active;
 	uint32_t ead_activity_flags;
 
-	old_active = !!CHECK_FLAG(evi_vtep->flags, BGP_EVPN_EVI_VTEP_ACTIVE);
+	old_active = CHECK_FLAG(evi_vtep->flags, BGP_EVPN_EVI_VTEP_ACTIVE);
 
 	if (bgp_mh_info->ead_evi_rx)
 		/* Both EAD-per-ES and EAD-per-EVI routes must be rxed from a PE
@@ -3135,7 +3135,7 @@ static void bgp_evpn_es_evi_vtep_re_eval_active(struct bgp *bgp,
 	else
 		UNSET_FLAG(evi_vtep->flags, BGP_EVPN_EVI_VTEP_ACTIVE);
 
-	new_active = !!CHECK_FLAG(evi_vtep->flags, BGP_EVPN_EVI_VTEP_ACTIVE);
+	new_active = CHECK_FLAG(evi_vtep->flags, BGP_EVPN_EVI_VTEP_ACTIVE);
 
 	if (old_active == new_active)
 		return;

bgpd/bgp_mpath.c

@@ -411,7 +411,8 @@ static void bgp_path_info_mpath_lb_update(struct bgp_path_info *path, bool set,
 {
 	struct bgp_path_info_mpath *mpath;
 
-	if ((mpath = path->mpath) == NULL) {
+	mpath = path->mpath;
+	if (mpath == NULL) {
 		if (!set || (cum_bw == 0 && !all_paths_lb))
 			return;
 

bgpd/bgp_nht.c

@@ -1053,7 +1053,7 @@ void evaluate_paths(struct bgp_nexthop_cache *bnc)
 		    || path->attr->srte_color != 0)
 			SET_FLAG(path->flags, BGP_PATH_IGP_CHANGED);
 
-		path_valid = !!CHECK_FLAG(path->flags, BGP_PATH_VALID);
+		path_valid = CHECK_FLAG(path->flags, BGP_PATH_VALID);
 		if (path_valid != bnc_is_valid_nexthop) {
 			if (path_valid) {
 				/* No longer valid, clear flag; also for EVPN

bgpd/bgp_route.c

@@ -11643,7 +11643,8 @@ static int bgp_show_route_in_table(struct vty *vty, struct bgp *bgp,
 			if (!table)
 				continue;
 
-			if ((rm = bgp_node_match(table, &match)) == NULL)
+			rm = bgp_node_match(table, &match);
+			if (rm == NULL)
 				continue;
 
 			const struct prefix *rm_p = bgp_dest_get_prefix(rm);
@@ -11735,7 +11736,8 @@ static int bgp_show_route_in_table(struct vty *vty, struct bgp *bgp,
 				json_object_free(json_paths);
 		}
 	} else {
-		if ((dest = bgp_node_match(rib, &match)) != NULL) {
+		dest = bgp_node_match(rib, &match);
+		if (dest != NULL) {
 			const struct prefix *dest_p = bgp_dest_get_prefix(dest);
 			if (!prefix_check
 			    || dest_p->prefixlen == match.prefixlen) {
@@ -14759,7 +14761,8 @@ static int bgp_clear_damp_route(struct vty *vty, const char *view_name,
 			table = bgp_dest_get_bgp_table_info(dest);
 			if (!table)
 				continue;
-			if ((rm = bgp_node_match(table, &match)) == NULL)
+			rm = bgp_node_match(table, &match);
+			if (rm == NULL)
 				continue;
 
 			const struct prefix *rm_p = bgp_dest_get_prefix(dest);
@@ -14783,8 +14786,8 @@ static int bgp_clear_damp_route(struct vty *vty, const char *view_name,
 			bgp_dest_unlock_node(rm);
 		}
 	} else {
-		if ((dest = bgp_node_match(bgp->rib[afi][safi], &match))
+		dest = bgp_node_match(bgp->rib[afi][safi], &match);
-		    != NULL) {
+		if (dest != NULL) {
 			const struct prefix *dest_p = bgp_dest_get_prefix(dest);
 
 			if (!prefix_check

bgpd/bgp_routemap_nb_config.c

@@ -70,8 +70,8 @@ static int bgp_route_match_delete(struct route_map_index *index,
 	if (type != RMAP_EVENT_MATCH_DELETED) {
 		/* ignore the mundane, the types without any dependency */
 		if (arg == NULL) {
-			if ((tmpstr = route_map_get_match_arg(index, command))
+			tmpstr = route_map_get_match_arg(index, command);
-					!= NULL)
+			if (tmpstr != NULL)
 				dep_name =
 					XSTRDUP(MTYPE_ROUTE_MAP_RULE, tmpstr);
 		} else {

bgpd/bgp_updgrp_adv.c

@@ -572,7 +572,8 @@ void bgp_adj_out_unset_subgroup(struct bgp_dest *dest,
 		return;
 
 	/* Lookup existing adjacency */
-	if ((adj = adj_lookup(dest, subgrp, addpath_tx_id)) != NULL) {
+	adj = adj_lookup(dest, subgrp, addpath_tx_id);
+	if (adj != NULL) {
 		/* Clean up previous advertisement.  */
 		if (adj->adv)
 			bgp_advertise_clean_subgroup(subgrp, adj);

bgpd/bgp_vty.c

@@ -1857,9 +1857,8 @@ void cli_show_router_bgp_med_config(struct vty *vty, struct lyd_node *dnode,
 		uint32_t med_admin_val;
 
 		vty_out(vty, " bgp max-med administrative");
-		if ((med_admin_val =
+		med_admin_val = yang_dnode_get_uint32(dnode, "./max-med-admin");
-			     yang_dnode_get_uint32(dnode, "./max-med-admin"))
+		if (med_admin_val != BGP_MAXMED_VALUE_DEFAULT)
-		    != BGP_MAXMED_VALUE_DEFAULT)
 			vty_out(vty, " %u", med_admin_val);
 		vty_out(vty, "\n");
 	}
@@ -10196,7 +10195,8 @@ static int bgp_clear_prefix(struct vty *vty, const char *view_name,
 			if (table == NULL)
 				continue;
 
-			if ((rm = bgp_node_match(table, &match)) != NULL) {
+			rm = bgp_node_match(table, &match);
+			if (rm != NULL) {
 				const struct prefix *rm_p =
 					bgp_dest_get_prefix(rm);
 
@@ -10209,7 +10209,8 @@ static int bgp_clear_prefix(struct vty *vty, const char *view_name,
 			}
 		}
 	} else {
-		if ((dest = bgp_node_match(rib, &match)) != NULL) {
+		dest = bgp_node_match(rib, &match);
+		if (dest != NULL) {
 			const struct prefix *dest_p = bgp_dest_get_prefix(dest);
 
 			if (dest_p->prefixlen == match.prefixlen) {

bgpd/bgpd.c

@@ -3099,8 +3099,7 @@ static struct bgp *bgp_create(as_t *as, const char *name,
 	afi_t afi;
 	safi_t safi;
 
-	if ((bgp = XCALLOC(MTYPE_BGP, sizeof(struct bgp))) == NULL)
+	bgp = XCALLOC(MTYPE_BGP, sizeof(struct bgp));
-		return NULL;
 
 	if (BGP_DEBUG(zebra, ZEBRA)) {
 		if (inst_type == BGP_INSTANCE_TYPE_DEFAULT)

lib/buffer.c

@@ -357,7 +357,8 @@ buffer_status_t buffer_flush_window(struct buffer *b, int fd, int width,
 
 			iov_size =
 				((iov_index > IOV_MAX) ? IOV_MAX : iov_index);
-			if ((nbytes = writev(fd, c_iov, iov_size)) < 0) {
+			nbytes = writev(fd, c_iov, iov_size);
+			if (nbytes < 0) {
 				flog_err(EC_LIB_SOCKET,
 					 "%s: writev to fd %d failed: %s",
 					 __func__, fd, safe_strerror(errno));
@@ -370,7 +371,8 @@ buffer_status_t buffer_flush_window(struct buffer *b, int fd, int width,
 		}
 	}
 #else  /* IOV_MAX */
-	if ((nbytes = writev(fd, iov, iov_index)) < 0)
+	nbytes = writev(fd, iov, iov_index);
+	if (nbytes < 0)
 		flog_err(EC_LIB_SOCKET, "%s: writev to fd %d failed: %s",
 			 __func__, fd, safe_strerror(errno));
 #endif /* IOV_MAX */
@@ -472,13 +474,17 @@ buffer_status_t buffer_write(struct buffer *b, int fd, const void *p,
 		/* Buffer is not empty, so do not attempt to write the new data.
 		 */
 		nbytes = 0;
-	else if ((nbytes = write(fd, p, size)) < 0) {
+	else {
-		if (ERRNO_IO_RETRY(errno))
+		nbytes = write(fd, p, size);
-			nbytes = 0;
+		if (nbytes < 0) {
-		else {
+			if (ERRNO_IO_RETRY(errno))
-			flog_err(EC_LIB_SOCKET, "%s: write error on fd %d: %s",
+				nbytes = 0;
-				 __func__, fd, safe_strerror(errno));
+			else {
-			return BUFFER_ERROR;
+				flog_err(EC_LIB_SOCKET,
+					 "%s: write error on fd %d: %s",
+					 __func__, fd, safe_strerror(errno));
+				return BUFFER_ERROR;
+			}
 		}
 	}
 	/* Add any remaining data to the buffer. */

lib/csv.c

@@ -641,7 +641,8 @@ static int get_memory_usage(pid_t pid)
 	char *vm;
 
 	snprintf(status_child, sizeof(status_child), "/proc/%d/status", pid);
-	if ((fd = open(status_child, O_RDONLY)) < 0)
+	fd = open(status_child, O_RDONLY);
+	if (fd < 0)
 		return -1;
 
 	read(fd, buf, 4095);

lib/link_state.c

@@ -538,8 +538,6 @@ struct ls_edge *ls_edge_add(struct ls_ted *ted,
 
 	/* Create Edge and add it to the TED */
 	new = XCALLOC(MTYPE_LS_DB, sizeof(struct ls_edge));
-	if (!new)
-		return NULL;
 
 	new->attributes = attributes;
 	new->key = key;
@@ -804,8 +802,6 @@ struct ls_ted *ls_ted_new(const uint32_t key, const char *name,
 	struct ls_ted *new;
 
 	new = XCALLOC(MTYPE_LS_DB, sizeof(struct ls_ted));
-	if (new == NULL)
-		return new;
 
 	/* Set basic information for this ted */
 	new->key = key;
@@ -1005,8 +1001,6 @@ static struct ls_node *ls_parse_node(struct stream *s)
 	size_t len;
 
 	node = XCALLOC(MTYPE_LS_DB, sizeof(struct ls_node));
-	if (node == NULL)
-		return NULL;
 
 	STREAM_GET(&node->adv, s, sizeof(struct ls_node_id));
 	STREAM_GETW(s, node->flags);
@@ -1051,8 +1045,6 @@ static struct ls_attributes *ls_parse_attributes(struct stream *s)
 	size_t len;
 
 	attr = XCALLOC(MTYPE_LS_DB, sizeof(struct ls_attributes));
-	if (attr == NULL)
-		return NULL;
 	attr->srlgs = NULL;
 
 	STREAM_GET(&attr->adv, s, sizeof(struct ls_node_id));
@@ -1157,8 +1149,6 @@ static struct ls_prefix *ls_parse_prefix(struct stream *s)
 	size_t len;
 
 	ls_pref = XCALLOC(MTYPE_LS_DB, sizeof(struct ls_prefix));
-	if (ls_pref == NULL)
-		return NULL;
 
 	STREAM_GET(&ls_pref->adv, s, sizeof(struct ls_node_id));
 	STREAM_GETW(s, ls_pref->flags);
@@ -1193,8 +1183,6 @@ struct ls_message *ls_parse_msg(struct stream *s)
 	struct ls_message *msg;
 
 	msg = XCALLOC(MTYPE_LS_DB, sizeof(struct ls_message));
-	if (msg == NULL)
-		return NULL;
 
 	/* Read LS Message header */
 	STREAM_GETC(s, msg->event);

lib/network.c

@@ -78,7 +78,8 @@ int set_nonblocking(int fd)
 	/* According to the Single UNIX Spec, the return value for F_GETFL
 	   should
 	   never be negative. */
-	if ((flags = fcntl(fd, F_GETFL)) < 0) {
+	flags = fcntl(fd, F_GETFL);
+	if (flags < 0) {
 		flog_err(EC_LIB_SYSTEM_CALL,
 			 "fcntl(F_GETFL) failed for fd %d: %s", fd,
 			 safe_strerror(errno));

lib/ntop.c

@@ -40,14 +40,18 @@ static inline void putbyte(uint8_t bytex, char **posx)
 	bool zero = false;
 	int byte = bytex, tmp, a, b;
 
-	if ((tmp = byte - 200) >= 0) {
+	tmp = byte - 200;
+	if (tmp >= 0) {
 		*pos++ = '2';
 		zero = true;
 		byte = tmp;
-	} else if ((tmp = byte - 100) >= 0) {
+	} else {
-		*pos++ = '1';
+		tmp = byte - 100;
-		zero = true;
+		if (tmp >= 0) {
-		byte = tmp;
+			*pos++ = '1';
+			zero = true;
+			byte = tmp;
+		}
 	}
 
 	/* make sure the compiler knows the value range of "byte" */

lib/prefix.c

@@ -1034,7 +1034,8 @@ const char *prefix2str(union prefixconstptr pu, char *str, int size)
 		l = strlen(buf);
 		buf[l++] = '/';
 		byte = p->prefixlen;
-		if ((tmp = p->prefixlen - 100) >= 0) {
+		tmp = p->prefixlen - 100;
+		if (tmp >= 0) {
 			buf[l++] = '1';
 			z = true;
 			byte = tmp;

lib/sockopt.c

@@ -379,14 +379,14 @@ static int setsockopt_ipv4_ifindex(int sock, ifindex_t val)
 	int ret;
 
 #if defined(IP_PKTINFO)
-	if ((ret = setsockopt(sock, IPPROTO_IP, IP_PKTINFO, &val, sizeof(val)))
+	ret = setsockopt(sock, IPPROTO_IP, IP_PKTINFO, &val, sizeof(val));
-	    < 0)
+	if (ret < 0)
 		flog_err(EC_LIB_SOCKET,
 			 "Can't set IP_PKTINFO option for fd %d to %d: %s",
 			 sock, val, safe_strerror(errno));
 #elif defined(IP_RECVIF)
-	if ((ret = setsockopt(sock, IPPROTO_IP, IP_RECVIF, &val, sizeof(val)))
+	ret = setsockopt(sock, IPPROTO_IP, IP_RECVIF, &val, sizeof(val));
-	    < 0)
+	if (ret < 0)
 		flog_err(EC_LIB_SOCKET,
 			 "Can't set IP_RECVIF option for fd %d to %d: %s", sock,
 			 val, safe_strerror(errno));
@@ -639,12 +639,8 @@ int sockopt_tcp_signature_ext(int sock, union sockunion *su, uint16_t prefixlen,
 
 #endif /* GNU_LINUX */
 
-	if ((ret = setsockopt(sock, IPPROTO_TCP, optname, &md5sig,
+	ret = setsockopt(sock, IPPROTO_TCP, optname, &md5sig, sizeof(md5sig));
-			      sizeof(md5sig)))
+	if (ret < 0) {
-	    < 0) {
-		/* ENOENT is harmless.  It is returned when we clear a password
-		   for which
-		   one was not previously set. */
 		if (ENOENT == errno)
 			ret = 0;
 		else

lib/stream.c

@@ -1097,7 +1097,8 @@ ssize_t stream_read_try(struct stream *s, int fd, size_t size)
 		return -1;
 	}
 
-	if ((nbytes = read(fd, s->data + s->endp, size)) >= 0) {
+	nbytes = read(fd, s->data + s->endp, size);
+	if (nbytes >= 0) {
 		s->endp += nbytes;
 		return nbytes;
 	}
@@ -1126,9 +1127,8 @@ ssize_t stream_recvfrom(struct stream *s, int fd, size_t size, int flags,
 		return -1;
 	}
 
-	if ((nbytes = recvfrom(fd, s->data + s->endp, size, flags, from,
+	nbytes = recvfrom(fd, s->data + s->endp, size, flags, from, fromlen);
-			       fromlen))
+	if (nbytes >= 0) {
-	    >= 0) {
 		s->endp += nbytes;
 		return nbytes;
 	}

lib/zclient.c

@@ -3812,7 +3812,8 @@ static int zclient_read(struct thread *thread)
 	zclient->t_read = NULL;
 
 	/* Read zebra header (if we don't have it already). */
-	if ((already = stream_get_endp(zclient->ibuf)) < ZEBRA_HEADER_SIZE) {
+	already = stream_get_endp(zclient->ibuf);
+	if (already < ZEBRA_HEADER_SIZE) {
 		ssize_t nbyte;
 		if (((nbyte = stream_read_try(zclient->ibuf, zclient->sock,
 					      ZEBRA_HEADER_SIZE - already))
@@ -3825,7 +3826,6 @@ static int zclient_read(struct thread *thread)
 			return zclient_failed(zclient);
 		}
 		if (nbyte != (ssize_t)(ZEBRA_HEADER_SIZE - already)) {
-			/* Try again later. */
 			zclient_event(ZCLIENT_READ, zclient);
 			return 0;
 		}

tools/coccinelle/xcalloc-xmalloc.cocci

@@ -0,0 +1,17 @@
+// No need checking against NULL for XMALLOC/XCALLOC.
+// If that happens, we have more problems with memory.
+
+@@
+type T;
+T *ptr;
+@@
+
+ptr =
+(
+XCALLOC(...)
+|
+XMALLOC(...)
+)
+...
+- if (ptr == NULL)
+- return ...;