From a43ad4fef85196592e59665fa9b69eddb30592e0 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Sat, 8 Sep 2018 19:25:00 +0200 Subject: [PATCH 1/4] lib, ldpd: fix SA warnings from TAILQ oddness Add a TAILQ_POP_FIRST so Clang understands it's the same item that is getting removed from the list. Signed-off-by: David Lamparter --- ldpd/lde.c | 4 +--- lib/imsg-buffer.c | 19 ++++++++++--------- lib/imsg.c | 3 +-- lib/queue.h | 13 +++++++++++++ 4 files changed, 25 insertions(+), 14 deletions(-) diff --git a/ldpd/lde.c b/ldpd/lde.c index 03b62b482b..8104398886 100644 --- a/ldpd/lde.c +++ b/ldpd/lde.c @@ -1620,10 +1620,8 @@ lde_address_list_free(struct lde_nbr *ln) { struct lde_addr *lde_addr; - while ((lde_addr = TAILQ_FIRST(&ln->addr_list)) != NULL) { - TAILQ_REMOVE(&ln->addr_list, lde_addr, entry); + while ((lde_addr = TAILQ_POP_FIRST(&ln->addr_list, entry)) != NULL) free(lde_addr); - } } static void zclient_sync_init(unsigned short instance) diff --git a/lib/imsg-buffer.c b/lib/imsg-buffer.c index b83f1f76f2..c2f4052b8f 100644 --- a/lib/imsg-buffer.c +++ b/lib/imsg-buffer.c @@ -21,9 +21,9 @@ #include "queue.h" #include "imsg.h" -int ibuf_realloc(struct ibuf *, size_t); -void ibuf_enqueue(struct msgbuf *, struct ibuf *); -void ibuf_dequeue(struct msgbuf *, struct ibuf *); +static int ibuf_realloc(struct ibuf *, size_t); +static void ibuf_enqueue(struct msgbuf *, struct ibuf *); +static void ibuf_dequeue(struct msgbuf *, struct ibuf *); struct ibuf *ibuf_open(size_t len) { @@ -57,7 +57,7 @@ struct ibuf *ibuf_dynamic(size_t len, size_t max) return (buf); } -int ibuf_realloc(struct ibuf *buf, size_t len) +static int ibuf_realloc(struct ibuf *buf, size_t len) { uint8_t *b; @@ -183,6 +183,8 @@ void msgbuf_drain(struct msgbuf *msgbuf, size_t n) next = TAILQ_NEXT(buf, entry); if (buf->rpos + n >= buf->wpos) { n -= buf->wpos - buf->rpos; + + TAILQ_REMOVE(&msgbuf->bufs, buf, entry); ibuf_dequeue(msgbuf, buf); } else { buf->rpos += n; @@ -195,7 +197,7 @@ void msgbuf_clear(struct msgbuf *msgbuf) { struct ibuf *buf; - while ((buf = TAILQ_FIRST(&msgbuf->bufs)) != NULL) + while ((buf = TAILQ_POP_FIRST(&msgbuf->bufs, entry)) != NULL) ibuf_dequeue(msgbuf, buf); } @@ -266,16 +268,15 @@ again: return (1); } -void ibuf_enqueue(struct msgbuf *msgbuf, struct ibuf *buf) +static void ibuf_enqueue(struct msgbuf *msgbuf, struct ibuf *buf) { TAILQ_INSERT_TAIL(&msgbuf->bufs, buf, entry); msgbuf->queued++; } -void ibuf_dequeue(struct msgbuf *msgbuf, struct ibuf *buf) +static void ibuf_dequeue(struct msgbuf *msgbuf, struct ibuf *buf) { - TAILQ_REMOVE(&msgbuf->bufs, buf, entry); - + /* TAILQ_REMOVE done by caller */ if (buf->fd != -1) close(buf->fd); diff --git a/lib/imsg.c b/lib/imsg.c index 5424140720..935d137727 100644 --- a/lib/imsg.c +++ b/lib/imsg.c @@ -299,11 +299,10 @@ int imsg_get_fd(struct imsgbuf *ibuf) int fd; struct imsg_fd *ifd; - if ((ifd = TAILQ_FIRST(&ibuf->fds)) == NULL) + if ((ifd = TAILQ_POP_FIRST(&ibuf->fds, entry)) == NULL) return (-1); fd = ifd->fd; - TAILQ_REMOVE(&ibuf->fds, ifd, entry); free(ifd); return (fd); diff --git a/lib/queue.h b/lib/queue.h index 04fbeee700..11e28b4c91 100644 --- a/lib/queue.h +++ b/lib/queue.h @@ -72,4 +72,17 @@ #include "freebsd-queue.h" #endif /* defined(__OpenBSD__) && !defined(STAILQ_HEAD) */ +#ifndef TAILQ_POP_FIRST +#define TAILQ_POP_FIRST(head, field) \ + ({ typeof((head)->tqh_first) _elm = TAILQ_FIRST(head); \ + if (_elm) { \ + if ((TAILQ_NEXT((_elm), field)) != NULL) \ + TAILQ_NEXT((_elm), field)->field.tqe_prev = \ + &TAILQ_FIRST(head); \ + else \ + (head)->tqh_last = &TAILQ_FIRST(head); \ + TAILQ_FIRST(head) = TAILQ_NEXT((_elm), field); \ + }; _elm; }) +#endif + #endif /* _FRR_QUEUE_H */ From f70247febe1e61bb77f9ba318a7dea55491d0b84 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Sat, 8 Sep 2018 19:47:05 +0200 Subject: [PATCH 2/4] lib: fix SA warning in skiplist code Clang was thinking the random level could be negative. (And, no, I couldn't figure that out by reading its output... trial and error this was.) Signed-off-by: David Lamparter --- lib/skiplist.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/skiplist.c b/lib/skiplist.c index a36bf47139..585cf859e5 100644 --- a/lib/skiplist.c +++ b/lib/skiplist.c @@ -202,6 +202,7 @@ int skiplist_insert(register struct skiplist *l, register void *key, } k = randomLevel(); + assert(k >= 0); if (k > l->level) { k = ++l->level; update[k] = l->header; From 4f4060f6abbfa004e0eb63b7c447776cc74c8d66 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Sat, 8 Sep 2018 20:16:59 +0200 Subject: [PATCH 3/4] *: fix clang-6 SA warnings I don't see these in CI, but my local clang-6 does emit warnings for these. Signed-off-by: David Lamparter --- lib/command.c | 1 + lib/csv.c | 2 ++ ospf6d/ospf6_asbr.c | 3 ++- ospf6d/ospf6_route.c | 2 +- ospfd/ospf_snmp.c | 1 - staticd/static_vty.c | 4 ++-- zebra/zebra_fpm_protobuf.c | 1 + 7 files changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/command.c b/lib/command.c index 1df6442107..3cc6806a88 100644 --- a/lib/command.c +++ b/lib/command.c @@ -1197,6 +1197,7 @@ static int handle_pipe_action(struct vty *vty, const char *cmd_in, /* retrieve action */ token = strsep(&working, " "); + assert(token); /* match result to known actions */ if (strmatch(token, "include")) { diff --git a/lib/csv.c b/lib/csv.c index ce84783aa6..2752974df5 100644 --- a/lib/csv.c +++ b/lib/csv.c @@ -563,6 +563,8 @@ void csv_decode(csv_t *csv, char *inbuf) csv_record_t *rec; buf = (inbuf) ? inbuf : csv->buf; + assert(buf); + pos = strpbrk(buf, "\n"); while (pos != NULL) { rec = calloc(1, sizeof(csv_record_t)); diff --git a/ospf6d/ospf6_asbr.c b/ospf6d/ospf6_asbr.c index 5af88defeb..dc7a3f6d45 100644 --- a/ospf6d/ospf6_asbr.c +++ b/ospf6d/ospf6_asbr.c @@ -732,7 +732,8 @@ void ospf6_asbr_lsa_remove(struct ospf6_lsa *lsa, ? 1 : 2, buf, listcount(route->paths), - listcount(route->nh_list)); + route->nh_list ? + listcount(route->nh_list) : 0); } if (listcount(route->paths)) { diff --git a/ospf6d/ospf6_route.c b/ospf6d/ospf6_route.c index a099eead49..021e825ae3 100644 --- a/ospf6d/ospf6_route.c +++ b/ospf6d/ospf6_route.c @@ -732,7 +732,7 @@ struct ospf6_route *ospf6_route_add(struct ospf6_route *route, route->next = next; if (node->info == next) { - assert(next->rnode == node); + assert(next && next->rnode == node); node->info = route; UNSET_FLAG(next->flag, OSPF6_ROUTE_BEST); SET_FLAG(route->flag, OSPF6_ROUTE_BEST); diff --git a/ospfd/ospf_snmp.c b/ospfd/ospf_snmp.c index 19d2e6a952..755634a2f1 100644 --- a/ospfd/ospf_snmp.c +++ b/ospfd/ospf_snmp.c @@ -995,7 +995,6 @@ static struct ospf_lsa *ospfLsdbLookup(struct variable *v, oid *name, if (len <= 0) type_next = 1; else { - len = 1; type_next = 0; *type = *offset; } diff --git a/staticd/static_vty.c b/staticd/static_vty.c index b323612d7e..f697969a72 100644 --- a/staticd/static_vty.c +++ b/staticd/static_vty.c @@ -88,8 +88,8 @@ static struct list *static_list; static int static_list_compare_helper(const char *s1, const char *s2) { - /* Are Both NULL */ - if (s1 == s2) + /* extra (!s1 && !s2) to keep SA happy */ + if (s1 == s2 || (!s1 && !s2)) return 0; if (!s1 && s2) diff --git a/zebra/zebra_fpm_protobuf.c b/zebra/zebra_fpm_protobuf.c index ebd632270c..be0f6a23be 100644 --- a/zebra/zebra_fpm_protobuf.c +++ b/zebra/zebra_fpm_protobuf.c @@ -129,6 +129,7 @@ static inline int add_nexthop(qpb_allocator_t *allocator, Fpm__AddRoute *msg, } // TODO: Use src. + (void)src; return 1; } From e10cfdaf51fd5662309eda11ff37dfac1d94abdf Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Sat, 8 Sep 2018 20:18:30 +0200 Subject: [PATCH 4/4] bfdd: fix garbage "port" string bfd_recv_ipv4() is getting an uninitialized buffer passed in as port, and then checks it without clearing it first. Thus we can end up leaving garbage data in it. Signed-off-by: David Lamparter --- bfdd/bfd_packet.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bfdd/bfd_packet.c b/bfdd/bfd_packet.c index 4bdfb314e2..8acb9438c5 100644 --- a/bfdd/bfd_packet.c +++ b/bfdd/bfd_packet.c @@ -248,6 +248,8 @@ ssize_t bfd_recv_ipv4(int sd, uint8_t *msgbuf, size_t msgbuflen, uint8_t *ttl, struct iovec iov[1]; uint8_t cmsgbuf[255]; + port[0] = '\0'; + /* Prepare the recvmsg params. */ iov[0].iov_base = msgbuf; iov[0].iov_len = msgbuflen;