diff --git a/Makefile.in b/Makefile.in index e6dbd572..da6cd206 100644 --- a/Makefile.in +++ b/Makefile.in @@ -184,6 +184,15 @@ check: tests tests_run tests: $(tests_targets) tests_run: $(tests_targets_ok) +STATIC_CHECKERS_ENABLE := nullability.NullableDereferenced nullability.NullablePassedToNonnull nullability.NullableReturnedFromNonnull optin.portability.UnixAPI valist.CopyToSelf valist.Uninitialized valist.Unterminated +STATIC_CHECKERS_DISABLE := deadcode.DeadStores +STATIC_SCAN_FLAGS := -o $(objdir)/static-scan/ $(addprefix -enable-checker ,$(STATIC_CHECKERS_ENABLE)) $(addprefix -disable-checker ,$(STATIC_CHECKERS_DISABLE)) + +static-scan: + $(E)echo Running static code analysis + $(Q)$(MAKE) clean + $(Q)scan-build $(STATIC_SCAN_FLAGS) $(MAKE) -$(MAKEFLAGS) + tags: cd $(srcdir) ; etags -lc `find $(dirs) -name *.[chY]` diff --git a/conf/cf-lex.l b/conf/cf-lex.l index 1d6cae2c..9ea05e9d 100644 --- a/conf/cf-lex.l +++ b/conf/cf-lex.l @@ -169,7 +169,7 @@ WHITE [ \t] errno = 0; l = bstrtoul10(yytext, &e); - if (e && (*e != ':') || (errno == ERANGE) || (l >> 32)) + if (!e || (*e != ':') || (errno == ERANGE) || (l >> 32)) cf_error("ASN out of range"); if (l >> 16) @@ -187,7 +187,7 @@ WHITE [ \t] errno = 0; l = bstrtoul10(e+1, &e); - if (e && *e || (errno == ERANGE) || (l >> len2)) + if (!e || *e || (errno == ERANGE) || (l >> len2)) cf_error("Number out of range"); cf_lval.i64 |= l; @@ -214,13 +214,13 @@ WHITE [ \t] errno = 0; l = bstrtoul10(yytext+2, &e); - if (e && (*e != ':') || (errno == ERANGE) || (l >> len1)) + if (!e || (*e != ':') || (errno == ERANGE) || (l >> len1)) cf_error("ASN out of range"); cf_lval.i64 |= ((u64) l) << len2; errno = 0; l = bstrtoul10(e+1, &e); - if (e && *e || (errno == ERANGE) || (l >> len2)) + if (!e || *e || (errno == ERANGE) || (l >> len2)) cf_error("Number out of range"); cf_lval.i64 |= l; @@ -242,7 +242,7 @@ WHITE [ \t] errno = 0; l = bstrtoul10(e, &e); - if (e && *e || (errno == ERANGE) || (l >> 16)) + if (!e || *e || (errno == ERANGE) || (l >> 16)) cf_error("Number out of range"); cf_lval.i64 |= l; @@ -266,7 +266,7 @@ WHITE [ \t] unsigned long int l; errno = 0; l = bstrtoul16(yytext+2, &e); - if (e && *e || errno == ERANGE || (unsigned long int)(unsigned int) l != l) + if (!e || *e || errno == ERANGE || (unsigned long int)(unsigned int) l != l) cf_error("Number out of range"); cf_lval.i = l; return NUM; @@ -277,7 +277,7 @@ WHITE [ \t] unsigned long int l; errno = 0; l = bstrtoul10(yytext, &e); - if (e && *e || errno == ERANGE || (unsigned long int)(unsigned int) l != l) + if (!e || *e || errno == ERANGE || (unsigned long int)(unsigned int) l != l) cf_error("Number out of range"); cf_lval.i = l; return NUM; diff --git a/configure.ac b/configure.ac index da8546a6..eabb3d56 100644 --- a/configure.ac +++ b/configure.ac @@ -24,6 +24,12 @@ AC_ARG_ENABLE([debug-generated], [enable_debug_generated=no] ) +AC_ARG_ENABLE([debug-expensive], + [AS_HELP_STRING([--enable-debug-expensive], [enable expensive consistency checks (implies --enable-debug) @<:@no@:>@])], + [], + [enable_debug_expensive=no] +) + AC_ARG_ENABLE([memcheck], [AS_HELP_STRING([--enable-memcheck], [check memory allocations when debugging @<:@yes@:>@])], [], @@ -72,6 +78,9 @@ AC_ARG_VAR([FLEX], [location of the Flex program]) AC_ARG_VAR([BISON], [location of the Bison program]) AC_ARG_VAR([M4], [location of the M4 program]) +if test "$enable_debug_expensive" = yes; then + enable_debug=yes +fi if test "$srcdir" = . ; then # Building in current directory => create obj directory holding all objects @@ -388,6 +397,10 @@ if test "$enable_debug" = yes ; then AC_CHECK_LIB([efence], [malloc]) fi fi + + if test "enable_debug_expensive" = yes ; then + AC_DEFINE([ENABLE_EXPENSIVE_CHECKS], [1], [Define to 1 if you want to run expensive consistency checks.]) + fi fi CLIENT=birdcl diff --git a/filter/config.Y b/filter/config.Y index 995f6cd4..49c59efc 100644 --- a/filter/config.Y +++ b/filter/config.Y @@ -185,159 +185,6 @@ f_generate_empty(struct f_dynamic_attr dyn) return f_new_inst(FI_EA_SET, f_new_inst(FI_CONSTANT, empty), dyn); } -#if 0 - -static inline struct f_inst * -f_generate_dpair(struct f_inst *t1, struct f_inst *t2) -{ - struct f_inst *rv; - - if ((t1->fi_code == FI_CONSTANT) && (t2->fi_code == FI_CONSTANT)) { - if ((t1->val.type != T_INT) || (t2->val.type != T_INT)) - cf_error( "Can't operate with value of non-integer type in pair constructor"); - - check_u16(t1->a[1].i); - check_u16(t2->a[1].i); - - rv = f_new_inst(FI_CONSTANT); - rv->val = (struct f_val) { - .type = T_PAIR, - .val.i = pair(t1->a[1].i, t2->a[1].i), - }; - } - else { - rv = f_new_inst(FI_PAIR_CONSTRUCT); - rv->a[0].p = t1; - rv->a[1].p = t2; - } - - return rv; -} - -static inline struct f_inst * -f_generate_ec(u16 kind, struct f_inst *tk, struct f_inst *tv) -{ - struct f_inst *rv; - int c1 = 0, c2 = 0, ipv4_used = 0; - u32 key = 0, val2 = 0; - - if (tk->fi_code == FI_CONSTANT) { - c1 = 1; - struct f_val *val = &(tk->val); - - if (val->type == T_INT) { - ipv4_used = 0; key = val->val.i; - } - else if (tk->val.type == T_QUAD) { - ipv4_used = 1; key = val->val.i; - } - else if ((val->type == T_IP) && ipa_is_ip4(val->val.ip)) { - ipv4_used = 1; key = ipa_to_u32(val->val.ip); - } - else - cf_error("Can't operate with key of non-integer/IPv4 type in EC constructor"); - } - - if (tv->fi_code == FI_CONSTANT) { - if (tv->val.type != T_INT) - cf_error("Can't operate with value of non-integer type in EC constructor"); - c2 = 1; - val2 = tv->val.val.i; - } - - if (c1 && c2) { - u64 ec; - - if (kind == EC_GENERIC) { - ec = ec_generic(key, val2); - } - else if (ipv4_used) { - check_u16(val2); - ec = ec_ip4(kind, key, val2); - } - else if (key < 0x10000) { - ec = ec_as2(kind, key, val2); - } - else { - check_u16(val2); - ec = ec_as4(kind, key, val2); - } - - rv = f_new_inst(FI_CONSTANT); - rv->val = (struct f_val) { - .type = T_EC, - .val.ec = ec, - }; - } - else { - rv = f_new_inst(FI_EC_CONSTRUCT); - rv->aux = kind; - rv->a[0].p = tk; - rv->a[1].p = tv; - } - - return rv; -} - -static inline struct f_inst * -f_generate_lc(struct f_inst *t1, struct f_inst *t2, struct f_inst *t3) -{ - struct f_inst *rv; - - if ((t1->fi_code == FI_CONSTANT) && (t2->fi_code == FI_CONSTANT) && (t3->fi_code == FI_CONSTANT)) { - if ((t1->val.type != T_INT) || (t2->val.type != T_INT) || (t3->val.type != T_INT)) - cf_error( "LC - Can't operate with value of non-integer type in tuple constructor"); - - rv = f_new_inst(FI_CONSTANT); - rv->val = (struct f_val) { - .type = T_LC, - .val.lc = (lcomm) { t1->a[1].i, t2->a[1].i, t3->a[1].i }, - }; - } - else - { - rv = f_new_inst(FI_LC_CONSTRUCT); - rv->a[0].p = t1; - rv->a[1].p = t2; - rv->a[2].p = t3; - } - - return rv; -} - -static inline struct f_inst * -f_generate_path_mask(struct f_inst *t) -{ - uint len = 0; - uint dyn = 0; - for (const struct f_inst *tt = t; tt; tt = tt->next) { - if (tt->fi_code != FI_CONSTANT) - dyn++; - len++; - } - - if (dyn) { - struct f_inst *pmc = f_new_inst(FI_PATHMASK_CONSTRUCT); - pmc->a[0].p = t; - pmc->a[1].i = len; - return pmc; - } - - struct f_path_mask *pm = cfg_allocz(sizeof(struct f_path_mask) + len * sizeof(struct f_path_mask_item)); - - uint i = 0; - for (const struct f_inst *tt = t; tt; tt = tt->next) - pm->item[i++] = tt->val.val.pmi; - - pm->len = i; - struct f_inst *pmc = f_new_inst(FI_CONSTANT); - pmc->val = (struct f_val) { .type = T_PATH_MASK, .val.path_mask = pm, }; - - return pmc; -} - -#endif - /* * Remove all new lines and doubled whitespaces * and convert all tabulators to spaces diff --git a/filter/decl.m4 b/filter/decl.m4 index efecb9a5..14b9329c 100644 --- a/filter/decl.m4 +++ b/filter/decl.m4 @@ -138,7 +138,7 @@ FID_IFCONST([[ } FID_IFCONST([[ const struct f_inst **items = NULL; - if (constargs) { + if (constargs && whati->varcount) { items = alloca(whati->varcount * sizeof(struct f_inst *)); const struct f_inst *child = fvar; for (uint i=0; child; i++) diff --git a/filter/f-inst.c b/filter/f-inst.c index 4b3c627b..3d185918 100644 --- a/filter/f-inst.c +++ b/filter/f-inst.c @@ -261,7 +261,7 @@ FID_MEMBER(enum ec_subtype, ecs, f1->ecs != f2->ecs, "ec subtype %s", ec_subtype_str(item->ecs)); - int check, ipv4_used; + int ipv4_used; u32 key, val; if (v1.type == T_INT) { @@ -279,21 +279,20 @@ val = v2.val.i; - if (ecs == EC_GENERIC) { - check = 0; RESULT(T_EC, ec, ec_generic(key, val)); - } - else if (ipv4_used) { - check = 1; RESULT(T_EC, ec, ec_ip4(ecs, key, val)); - } - else if (key < 0x10000) { - check = 0; RESULT(T_EC, ec, ec_as2(ecs, key, val)); - } - else { - check = 1; RESULT(T_EC, ec, ec_as4(ecs, key, val)); - } - - if (check && (val > 0xFFFF)) - runtime("Value %u > %u out of bounds in EC constructor", val, 0xFFFF); + if (ecs == EC_GENERIC) + RESULT(T_EC, ec, ec_generic(key, val)); + else if (ipv4_used) + if (val <= 0xFFFF) + RESULT(T_EC, ec, ec_ip4(ecs, key, val)); + else + runtime("4-byte value %u can't be used with IP-address key in extended community", val); + else if (key < 0x10000) + RESULT(T_EC, ec, ec_as2(ecs, key, val)); + else + if (val <= 0xFFFF) + RESULT(T_EC, ec, ec_as4(ecs, key, val)); + else + runtime("4-byte value %u can't be used with 4-byte ASN in extended community", val); } INST(FI_LC_CONSTRUCT, 3, 1) { diff --git a/lib/birdlib.h b/lib/birdlib.h index 5202b0c8..23036c1b 100644 --- a/lib/birdlib.h +++ b/lib/birdlib.h @@ -72,6 +72,7 @@ static inline int u64_cmp(u64 i1, u64 i2) #define NORET __attribute__((noreturn)) #define UNUSED __attribute__((unused)) #define PACKED __attribute__((packed)) +#define NONNULL(...) __attribute__((nonnull((__VA_ARGS__)))) #ifndef HAVE_THREAD_LOCAL #define _Thread_local @@ -162,12 +163,23 @@ void debug(const char *msg, ...); /* Printf to debug output */ #define DBG(x, y...) do { } while(0) #endif +#define ASSERT_DIE(x) do { if (!(x)) bug("Assertion '%s' failed at %s:%d", #x, __FILE__, __LINE__); } while(0) + +#define EXPENSIVE_CHECK(x) /* intentionally left blank */ + #ifdef DEBUGGING -#define ASSERT(x) do { if (!(x)) bug("Assertion '%s' failed at %s:%d", #x, __FILE__, __LINE__); } while(0) +#define ASSERT(x) ASSERT_DIE(x) +#define ASSUME(x) ASSERT_DIE(x) +#ifdef ENABLE_EXPENSIVE_CHECKS +#undef EXPENSIVE_CHECK +#define EXPENSIVE_CHECK(x) ASSERT_DIE(x) +#endif #else #define ASSERT(x) do { if (!(x)) log(L_BUG "Assertion '%s' failed at %s:%d", #x, __FILE__, __LINE__); } while(0) +#define ASSUME(x) /* intentionally left blank */ #endif + #ifdef DEBUGGING asm( ".pushsection \".debug_gdb_scripts\", \"MS\",@progbits,1\n" diff --git a/lib/ip.c b/lib/ip.c index 2d195160..fcc72caf 100644 --- a/lib/ip.c +++ b/lib/ip.c @@ -264,6 +264,9 @@ ip6_pton(const char *a, ip6_addr *o) int i, j, k, l, hfil; const char *start; + if (!a[0]) /* Empty string check */ + return 0; + if (a[0] == ':') /* Leading :: */ { if (a[1] != ':') @@ -333,6 +336,8 @@ ip6_pton(const char *a, ip6_addr *o) for (; i>=hfil; i--) words[i] = 0; } + else if (i != 8) /* Incomplete address */ + return 0; /* Convert the address to ip6_addr format */ for (i=0; i<4; i++) diff --git a/lib/ip_test.c b/lib/ip_test.c index fd70c957..36d10d68 100644 --- a/lib/ip_test.c +++ b/lib/ip_test.c @@ -13,25 +13,38 @@ #define IP4_MAX_LEN 16 static int -test_ipa_pton(void *out_, const void *in_, const void *expected_out_) +test_ip4_pton(void *out_, const void *in_, const void *expected_out_) +{ + ip_addr *out = out_; + const char *in = in_; + const ip_addr *expected_out = expected_out_; + ip4_addr ip4; + + if (expected_out) + { + bt_assert(ip4_pton(in, &ip4)); + *out = ipa_from_ip4(ip4); + return ipa_equal(*out, *expected_out); + } + else + return !ip4_pton(in, &ip4); + +} + +static int +test_ip6_pton(void *out_, const void *in_, const void *expected_out_) { ip_addr *out = out_; const char *in = in_; const ip_addr *expected_out = expected_out_; - if (ipa_is_ip4(*expected_out)) - { - ip4_addr ip4; - bt_assert(ip4_pton(in, &ip4)); - *out = ipa_from_ip4(ip4); - } - else + if (expected_out) { bt_assert(ip6_pton(in, out)); - /* ip_addr == ip6_addr */ + return ipa_equal(*out, *expected_out); } - - return ipa_equal(*out, *expected_out); + else + return !ip6_pton(in, out); } static int @@ -52,7 +65,7 @@ t_ip4_pton(void) }, }; - return bt_assert_batch(test_vectors, test_ipa_pton, bt_fmt_str, bt_fmt_ipa); + return bt_assert_batch(test_vectors, test_ip4_pton, bt_fmt_str, bt_fmt_ipa); } static int @@ -87,9 +100,17 @@ t_ip6_pton(void) .in = "2605:2700:0:3::4713:93e3", .out = & ipa_build6(0x26052700, 0x00000003, 0x00000000, 0x471393E3), }, + { + .in = "2605:2700:0:3:4713:93e3", + .out = NULL, + }, + { + .in = "2", + .out = NULL, + }, }; - return bt_assert_batch(test_vectors, test_ipa_pton, bt_fmt_str, bt_fmt_ipa); + return bt_assert_batch(test_vectors, test_ip6_pton, bt_fmt_str, bt_fmt_ipa); } static int diff --git a/lib/lists.c b/lib/lists.c index 4a48d3b7..200576cf 100644 --- a/lib/lists.c +++ b/lib/lists.c @@ -29,6 +29,42 @@ #include "nest/bird.h" #include "lib/lists.h" +LIST_INLINE int +check_list(list *l, node *n) +{ + if (!l) + { + ASSERT_DIE(n); + ASSERT_DIE(n->prev); + + do { n = n->prev; } while (n->prev); + + l = SKIP_BACK(list, head_node, n); + } + + int seen = 0; + + ASSERT_DIE(l->null == NULL); + ASSERT_DIE(l->head != NULL); + ASSERT_DIE(l->tail != NULL); + + node *prev = &l->head_node, *cur = l->head, *next = l->head->next; + while (next) + { + if (cur == n) + seen++; + ASSERT_DIE(cur->prev == prev); + prev = cur; + cur = next; + next = next->next; + } + + ASSERT_DIE(cur == &(l->tail_node)); + ASSERT_DIE(!n || (seen == 1)); + + return 1; +} + /** * add_tail - append a node to a list * @l: linked list @@ -39,6 +75,10 @@ LIST_INLINE void add_tail(list *l, node *n) { + EXPENSIVE_CHECK(check_list(l, NULL)); + ASSUME(n->prev == NULL); + ASSUME(n->next == NULL); + node *z = l->tail; n->next = &l->tail_node; @@ -57,6 +97,10 @@ add_tail(list *l, node *n) LIST_INLINE void add_head(list *l, node *n) { + EXPENSIVE_CHECK(check_list(l, NULL)); + ASSUME(n->prev == NULL); + ASSUME(n->next == NULL); + node *z = l->head; n->next = z; @@ -76,6 +120,10 @@ add_head(list *l, node *n) LIST_INLINE void insert_node(node *n, node *after) { + EXPENSIVE_CHECK(check_list(l, after)); + ASSUME(n->prev == NULL); + ASSUME(n->next == NULL); + node *z = after->next; n->next = z; @@ -93,6 +141,8 @@ insert_node(node *n, node *after) LIST_INLINE void rem_node(node *n) { + EXPENSIVE_CHECK(check_list(NULL, n)); + node *z = n->prev; node *x = n->next; @@ -103,24 +153,20 @@ rem_node(node *n) } /** - * replace_node - replace a node in a list with another one - * @old: node to be removed - * @new: node to be inserted + * update_node - update node after calling realloc on it + * @n: node to be updated * - * Replaces node @old in the list it's linked in with node @new. Node - * @old may be a copy of the original node, which is not accessed - * through the list. The function could be called with @old == @new, - * which just fixes neighbors' pointers in the case that the node - * was reallocated. + * Fixes neighbor pointers. */ LIST_INLINE void -replace_node(node *old, node *new) +update_node(node *n) { - old->next->prev = new; - old->prev->next = new; + ASSUME(n->next->prev == n->prev->next); - new->prev = old->prev; - new->next = old->next; + n->next->prev = n; + n->prev->next = n; + + EXPENSIVE_CHECK(check_list(NULL, n)); } /** @@ -149,6 +195,9 @@ init_list(list *l) LIST_INLINE void add_tail_list(list *to, list *l) { + EXPENSIVE_CHECK(check_list(to, NULL)); + EXPENSIVE_CHECK(check_list(l, NULL)); + node *p = to->tail; node *q = l->head; @@ -157,6 +206,8 @@ add_tail_list(list *to, list *l) q = l->tail; q->next = &to->tail_node; to->tail = q; + + EXPENSIVE_CHECK(check_list(to, NULL)); } LIST_INLINE uint @@ -165,6 +216,8 @@ list_length(list *l) uint len = 0; node *n; + EXPENSIVE_CHECK(check_list(l, NULL)); + WALK_LIST(n, *l) len++; diff --git a/lib/lists_test.c b/lib/lists_test.c index f26a88e2..cf0021fe 100644 --- a/lib/lists_test.c +++ b/lib/lists_test.c @@ -222,26 +222,29 @@ t_remove_node(void) } static int -t_replace_node(void) +t_update_node(void) { node head, inside, tail; init_list_(); fill_list(); - replace_node(&nodes[0], &head); + head = nodes[0]; + update_node(&head); bt_assert(l.head == &head); bt_assert(head.prev == NODE &l.head); bt_assert(head.next == &nodes[1]); bt_assert(nodes[1].prev == &head); - replace_node(&nodes[MAX_NUM/2], &inside); + inside = nodes[MAX_NUM/2]; + update_node(&inside); bt_assert(nodes[MAX_NUM/2-1].next == &inside); bt_assert(nodes[MAX_NUM/2+1].prev == &inside); bt_assert(inside.prev == &nodes[MAX_NUM/2-1]); bt_assert(inside.next == &nodes[MAX_NUM/2+1]); - replace_node(&nodes[MAX_NUM-1], &tail); + tail = nodes[MAX_NUM-1]; + update_node(&tail); bt_assert(l.tail == &tail); bt_assert(tail.prev == &nodes[MAX_NUM-2]); bt_assert(tail.next == NODE &l.null); @@ -280,7 +283,7 @@ main(int argc, char *argv[]) bt_test_suite(t_add_head, "Adding nodes to head of list"); bt_test_suite(t_insert_node, "Inserting nodes to list"); bt_test_suite(t_remove_node, "Removing nodes from list"); - bt_test_suite(t_replace_node, "Replacing nodes in list"); + bt_test_suite(t_update_node, "Updating nodes in list"); bt_test_suite(t_add_tail_list, "At the tail of a list adding the another list"); return bt_exit_value(); diff --git a/lib/resource.c b/lib/resource.c index ab8c800f..5589373e 100644 --- a/lib/resource.c +++ b/lib/resource.c @@ -340,6 +340,7 @@ mb_alloc(pool *p, unsigned size) struct mblock *b = xmalloc(sizeof(struct mblock) + size); b->r.class = &mb_class; + b->r.n = (node) {}; add_tail(&p->inside, &b->r.n); b->size = size; return b->data; @@ -387,7 +388,7 @@ mb_realloc(void *m, unsigned size) struct mblock *b = SKIP_BACK(struct mblock, data, m); b = xrealloc(b, sizeof(struct mblock) + size); - replace_node(&b->r.n, &b->r.n); + update_node(&b->r.n); b->size = size; return b->data; } diff --git a/lib/slab.c b/lib/slab.c index 5c414f9e..c3035b45 100644 --- a/lib/slab.c +++ b/lib/slab.c @@ -216,8 +216,11 @@ sl_new_head(slab *s) struct sl_obj *no; uint n = s->objs_per_slab; - h->first_free = o; - h->num_full = 0; + *h = (struct sl_head) { + .first_free = o, + .num_full = 0, + }; + while (n--) { o->slab = h; diff --git a/lib/string.h b/lib/string.h index d6ae5ef7..0f650178 100644 --- a/lib/string.h +++ b/lib/string.h @@ -72,6 +72,15 @@ bstrcmp(const char *s1, const char *s2) return !s2 - !s1; } +static inline void * +bmemcpy(void *dest, const void *src, size_t n) +{ + if (n) + return memcpy(dest, src, n); + else + return dest; +} + #define ROUTER_ID_64_LENGTH 23 #endif diff --git a/lib/timer.c b/lib/timer.c index be695114..a5abbcc4 100644 --- a/lib/timer.c +++ b/lib/timer.c @@ -255,7 +255,7 @@ timer_init(void) btime tm_parse_time(const char *x) { - struct tm tm; + struct tm tm = {}; int usec, n1, n2, n3, r; r = sscanf(x, "%d-%d-%d%n %d:%d:%d%n.%d%n", diff --git a/nest/protocol.h b/nest/protocol.h index e97e59dd..a934c047 100644 --- a/nest/protocol.h +++ b/nest/protocol.h @@ -80,7 +80,7 @@ struct protocol { void (*cleanup)(struct proto *); /* Called after shutdown when protocol became hungry/down */ void (*get_status)(struct proto *, byte *buf); /* Get instance status (for `show protocols' command) */ void (*get_route_info)(struct rte *, byte *buf); /* Get route information (for `show route' command) */ - int (*get_attr)(struct eattr *, byte *buf, int buflen); /* ASCIIfy dynamic attribute (returns GA_*) */ + int (*get_attr)(const struct eattr *, byte *buf, int buflen); /* ASCIIfy dynamic attribute (returns GA_*) */ void (*show_proto_info)(struct proto *); /* Show protocol info (for `show protocols all' command) */ void (*copy_config)(struct proto_config *, struct proto_config *); /* Copy config from given protocol instance */ }; diff --git a/nest/route.h b/nest/route.h index 5421ece5..1b4f2866 100644 --- a/nest/route.h +++ b/nest/route.h @@ -577,7 +577,7 @@ void ea_merge(ea_list *from, ea_list *to); /* Merge sub-lists to allocated buffe int ea_same(ea_list *x, ea_list *y); /* Test whether two ea_lists are identical */ uint ea_hash(ea_list *e); /* Calculate 16-bit hash value */ ea_list *ea_append(ea_list *to, ea_list *what); -void ea_format_bitfield(struct eattr *a, byte *buf, int bufsize, const char **names, int min, int max); +void ea_format_bitfield(const struct eattr *a, byte *buf, int bufsize, const char **names, int min, int max); #define ea_normalize(ea) do { \ if (ea->next) { \ diff --git a/nest/rt-attr.c b/nest/rt-attr.c index 8620d321..28d956bc 100644 --- a/nest/rt-attr.c +++ b/nest/rt-attr.c @@ -202,7 +202,7 @@ nexthop__same(struct nexthop *x, struct nexthop *y) } static int -nexthop_compare_node(const struct nexthop *x, const struct nexthop *y) +nexthop_compare_node(const struct nexthop *x, const struct nexthop *y) { int r; @@ -278,18 +278,22 @@ nexthop_merge(struct nexthop *x, struct nexthop *y, int rx, int ry, int max, lin while ((x || y) && max--) { int cmp = nexthop_compare_node(x, y); + if (cmp < 0) { + ASSUME(x); *n = rx ? x : nexthop_copy_node(x, lp); x = x->next; } else if (cmp > 0) { + ASSUME(y); *n = ry ? y : nexthop_copy_node(y, lp); y = y->next; } else { + ASSUME(x && y); *n = rx ? x : (ry ? y : nexthop_copy_node(x, lp)); x = x->next; y = y->next; @@ -786,7 +790,7 @@ ea_free(ea_list *o) } static int -get_generic_attr(eattr *a, byte **buf, int buflen UNUSED) +get_generic_attr(const eattr *a, byte **buf, int buflen UNUSED) { if (a->id == EA_GEN_IGP_METRIC) { @@ -798,7 +802,7 @@ get_generic_attr(eattr *a, byte **buf, int buflen UNUSED) } void -ea_format_bitfield(struct eattr *a, byte *buf, int bufsize, const char **names, int min, int max) +ea_format_bitfield(const struct eattr *a, byte *buf, int bufsize, const char **names, int min, int max) { byte *bound = buf + bufsize - 32; u32 data = a->u.data; @@ -894,7 +898,7 @@ ea_show_lc_set(struct cli *c, const struct adata *ad, byte *pos, byte *buf, byte * get_attr() hook, it's consulted first. */ void -ea_show(struct cli *c, eattr *e) +ea_show(struct cli *c, const eattr *e) { struct protocol *p; int status = GA_UNKNOWN; diff --git a/nest/rt-show.c b/nest/rt-show.c index 3431293a..bd0df9ee 100644 --- a/nest/rt-show.c +++ b/nest/rt-show.c @@ -104,6 +104,12 @@ rt_show_net(struct cli *c, net *n, struct rt_show_data *d) rte *e, *ee; byte ia[NET_MAX_TEXT_LENGTH+1]; struct channel *ec = d->tab->export_channel; + + /* The Clang static analyzer complains that ec may be NULL. + * It should be ensured to be not NULL by rt_show_prepare_tables() */ + if (d->export_mode) + ASSUME(ec); + int first = 1; int pass = 0; diff --git a/nest/rt-table.c b/nest/rt-table.c index a46eeb77..ae5a8444 100644 --- a/nest/rt-table.c +++ b/nest/rt-table.c @@ -2304,7 +2304,7 @@ rt_commit(struct config *new, struct config *old) WALK_LIST(r, new->tables) if (!r->table) { - rtable *t = mb_alloc(rt_table_pool, sizeof(struct rtable)); + rtable *t = mb_allocz(rt_table_pool, sizeof(struct rtable)); DBG("\t%s: created\n", r->name); rt_setup(rt_table_pool, t, r); add_tail(&routing_tables, &t->n); diff --git a/proto/babel/babel.c b/proto/babel/babel.c index a915e8fa..ebd5f7cc 100644 --- a/proto/babel/babel.c +++ b/proto/babel/babel.c @@ -1852,7 +1852,7 @@ babel_get_route_info(rte *rte, byte *buf) } static int -babel_get_attr(eattr *a, byte *buf, int buflen UNUSED) +babel_get_attr(const eattr *a, byte *buf, int buflen UNUSED) { switch (a->id) { diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c index 655ddb62..4710bfba 100644 --- a/proto/bgp/attrs.c +++ b/proto/bgp/attrs.c @@ -72,7 +72,7 @@ struct bgp_attr_desc { void (*export)(struct bgp_export_state *s, eattr *a); int (*encode)(struct bgp_write_state *s, eattr *a, byte *buf, uint size); void (*decode)(struct bgp_parse_state *s, uint code, uint flags, byte *data, uint len, ea_list **to); - void (*format)(eattr *ea, byte *buf, uint size); + void (*format)(const eattr *ea, byte *buf, uint size); }; static const struct bgp_attr_desc bgp_attr_table[]; @@ -396,7 +396,7 @@ bgp_decode_origin(struct bgp_parse_state *s, uint code UNUSED, uint flags, byte } static void -bgp_format_origin(eattr *a, byte *buf, uint size UNUSED) +bgp_format_origin(const eattr *a, byte *buf, uint size UNUSED) { static const char *bgp_origin_names[] = { "IGP", "EGP", "Incomplete" }; @@ -510,7 +510,7 @@ bgp_decode_next_hop(struct bgp_parse_state *s, uint code UNUSED, uint flags UNUS /* TODO: This function should use AF-specific hook */ static void -bgp_format_next_hop(eattr *a, byte *buf, uint size UNUSED) +bgp_format_next_hop(const eattr *a, byte *buf, uint size UNUSED) { ip_addr *nh = (void *) a->u.ptr->data; uint len = a->u.ptr->length; @@ -601,7 +601,7 @@ bgp_decode_aggregator(struct bgp_parse_state *s, uint code UNUSED, uint flags, b } static void -bgp_format_aggregator(eattr *a, byte *buf, uint size UNUSED) +bgp_format_aggregator(const eattr *a, byte *buf, uint size UNUSED) { const byte *data = a->u.ptr->data; @@ -676,7 +676,7 @@ bgp_decode_cluster_list(struct bgp_parse_state *s, uint code UNUSED, uint flags, } static void -bgp_format_cluster_list(eattr *a, byte *buf, uint size) +bgp_format_cluster_list(const eattr *a, byte *buf, uint size) { /* Truncates cluster lists larger than buflen, probably not a problem */ int_set_format(a->u.ptr, 0, -1, buf, size); @@ -831,7 +831,7 @@ bgp_decode_aigp(struct bgp_parse_state *s, uint code UNUSED, uint flags, byte *d } static void -bgp_format_aigp(eattr *a, byte *buf, uint size UNUSED) +bgp_format_aigp(const eattr *a, byte *buf, uint size UNUSED) { const byte *b = bgp_aigp_get_tlv(a->u.ptr, BGP_AIGP_METRIC); @@ -909,7 +909,7 @@ bgp_decode_mpls_label_stack(struct bgp_parse_state *s, uint code UNUSED, uint fl } static void -bgp_format_mpls_label_stack(eattr *a, byte *buf, uint size) +bgp_format_mpls_label_stack(const eattr *a, byte *buf, uint size) { u32 *labels = (u32 *) a->u.ptr->data; uint lnum = a->u.ptr->length / 4; @@ -2293,7 +2293,7 @@ bgp_process_as4_attrs(ea_list **attrs, struct linpool *pool) } int -bgp_get_attr(eattr *a, byte *buf, int buflen) +bgp_get_attr(const eattr *a, byte *buf, int buflen) { uint i = EA_ID(a->id); const struct bgp_attr_desc *d; diff --git a/proto/bgp/bgp.h b/proto/bgp/bgp.h index dc63e13e..455f04f9 100644 --- a/proto/bgp/bgp.h +++ b/proto/bgp/bgp.h @@ -552,7 +552,7 @@ static inline void bgp_set_attr_data(ea_list **to, struct linpool *pool, uint code, uint flags, void *data, uint len) { struct adata *a = lp_alloc_adata(pool, len); - memcpy(a->data, data, len); + bmemcpy(a->data, data, len); bgp_set_attr(to, pool, code, flags, (uintptr_t) a); } @@ -581,7 +581,7 @@ int bgp_rte_recalculate(rtable *table, net *net, rte *new, rte *old, rte *old_be struct rte *bgp_rte_modify_stale(struct rte *r, struct linpool *pool); void bgp_rt_notify(struct proto *P, struct channel *C, net *n, rte *new, rte *old); int bgp_preexport(struct proto *, struct rte **, struct linpool *); -int bgp_get_attr(struct eattr *e, byte *buf, int buflen); +int bgp_get_attr(const struct eattr *e, byte *buf, int buflen); void bgp_get_route_info(struct rte *, byte *buf); int bgp_total_aigp_metric_(rte *e, u64 *metric, const struct adata **ad); diff --git a/proto/ospf/neighbor.c b/proto/ospf/neighbor.c index 30e80640..18692d6e 100644 --- a/proto/ospf/neighbor.c +++ b/proto/ospf/neighbor.c @@ -650,19 +650,20 @@ void ospf_dr_election(struct ospf_iface *ifa) { struct ospf_proto *p = ifa->oa->po; - struct ospf_neighbor *neigh, *ndr, *nbdr, me; + struct ospf_neighbor *neigh, *ndr, *nbdr; u32 myid = p->router_id; DBG("(B)DR election.\n"); - me.state = NEIGHBOR_2WAY; - me.rid = myid; - me.priority = ifa->priority; - me.ip = ifa->addr->ip; - - me.dr = ospf_is_v2(p) ? ipa_to_u32(ifa->drip) : ifa->drid; - me.bdr = ospf_is_v2(p) ? ipa_to_u32(ifa->bdrip) : ifa->bdrid; - me.iface_id = ifa->iface_id; + struct ospf_neighbor me = { + .state = NEIGHBOR_2WAY, + .rid = myid, + .priority = ifa->priority, + .ip = ifa->addr->ip, + .dr = ospf_is_v2(p) ? ipa_to_u32(ifa->drip) : ifa->drid, + .bdr = ospf_is_v2(p) ? ipa_to_u32(ifa->bdrip) : ifa->bdrid, + .iface_id = ifa->iface_id, + }; add_tail(&ifa->neigh_list, NODE & me); diff --git a/proto/ospf/ospf.c b/proto/ospf/ospf.c index 29610f4a..c8ed0e06 100644 --- a/proto/ospf/ospf.c +++ b/proto/ospf/ospf.c @@ -620,7 +620,7 @@ ospf_get_route_info(rte * rte, byte * buf) } static int -ospf_get_attr(eattr * a, byte * buf, int buflen UNUSED) +ospf_get_attr(const eattr * a, byte * buf, int buflen UNUSED) { switch (a->id) { @@ -1244,7 +1244,7 @@ ospf_sh_state(struct proto *P, int verbose, int reachable) uint num = p->gr->hash_entries; struct top_hash_entry *hea[num]; - struct top_hash_entry *hex[verbose ? num : 0]; + struct top_hash_entry **hex = verbose ? alloca(num * sizeof(struct top_hash_entry *)) : NULL; struct top_hash_entry *he; struct top_hash_entry *cnode = NULL; @@ -1289,7 +1289,9 @@ ospf_sh_state(struct proto *P, int verbose, int reachable) lsa_compare_ospf3 = !ospf2; qsort(hea, j1, sizeof(struct top_hash_entry *), lsa_compare_for_state); - qsort(hex, jx, sizeof(struct top_hash_entry *), ext_compare_for_state); + + if (verbose) + qsort(hex, jx, sizeof(struct top_hash_entry *), ext_compare_for_state); /* * This code is a bit tricky, we have a primary LSAs (router and diff --git a/proto/ospf/topology.c b/proto/ospf/topology.c index 2e9c3965..c8ec730a 100644 --- a/proto/ospf/topology.c +++ b/proto/ospf/topology.c @@ -329,6 +329,14 @@ ospf_originate_lsa(struct ospf_proto *p, struct ospf_new_lsa *lsa) en->next_lsa_opts = 0; } + /* The static analyzer complains here that en->lsa_body may be NULL. + * Yes, it may if ospf_hash_get() creates a new struct top_hash_entry. + * In this case, also en->lsa.length must be 0 and lsa_length is never + * equal to 0 while sizeof(struct ospf_lsa_header) is non-zero. + * Therefore memcmp() is never executed with NULL here. + * */ + ASSUME((en->lsa.length == 0) == (en->lsa_body == NULL)); + /* Ignore the the new LSA if is the same as the current one */ if ((en->lsa.age < LSA_MAXAGE) && (lsa_length == en->lsa.length) && diff --git a/proto/radv/radv.c b/proto/radv/radv.c index 622b3c3c..b4235917 100644 --- a/proto/radv/radv.c +++ b/proto/radv/radv.c @@ -740,7 +740,7 @@ radv_pref_str(u32 pref) /* The buffer has some minimal size */ static int -radv_get_attr(eattr *a, byte *buf, int buflen UNUSED) +radv_get_attr(const eattr *a, byte *buf, int buflen UNUSED) { switch (a->id) { diff --git a/proto/rip/rip.c b/proto/rip/rip.c index f02d5071..ae8007d9 100644 --- a/proto/rip/rip.c +++ b/proto/rip/rip.c @@ -1190,7 +1190,7 @@ rip_get_route_info(rte *rte, byte *buf) } static int -rip_get_attr(eattr *a, byte *buf, int buflen UNUSED) +rip_get_attr(const eattr *a, byte *buf, int buflen UNUSED) { switch (a->id) { diff --git a/proto/rpki/packets.c b/proto/rpki/packets.c index 59a5efaf..e9d24fb8 100644 --- a/proto/rpki/packets.c +++ b/proto/rpki/packets.c @@ -1011,6 +1011,7 @@ rpki_send_error_pdu(struct rpki_cache *cache, const enum pdu_error_type error_co { va_start(args, fmt); msg_len = bvsnprintf(msg, sizeof(msg), fmt, args) + 1; + va_end(args); } u32 pdu_size = 16 + err_pdu_len + msg_len; diff --git a/sysdep/bsd/krt-sys.h b/sysdep/bsd/krt-sys.h index aa6cc72e..57501884 100644 --- a/sysdep/bsd/krt-sys.h +++ b/sysdep/bsd/krt-sys.h @@ -46,7 +46,7 @@ static inline void krt_sys_io_init(void) { } static inline void krt_sys_init(struct krt_proto *p UNUSED) { } static inline void krt_sys_postconfig(struct krt_config *x UNUSED) { } -static inline int krt_sys_get_attr(eattr *a UNUSED, byte *buf UNUSED, int buflen UNUSED) { return GA_UNKNOWN; } +static inline int krt_sys_get_attr(const eattr *a UNUSED, byte *buf UNUSED, int buflen UNUSED) { return GA_UNKNOWN; } #endif diff --git a/sysdep/linux/netlink.c b/sysdep/linux/netlink.c index 856869ac..a9e711b4 100644 --- a/sysdep/linux/netlink.c +++ b/sysdep/linux/netlink.c @@ -2065,7 +2065,7 @@ static const char *krt_features_names[KRT_FEATURES_MAX] = { }; int -krt_sys_get_attr(eattr *a, byte *buf, int buflen UNUSED) +krt_sys_get_attr(const eattr *a, byte *buf, int buflen UNUSED) { switch (a->id) { diff --git a/sysdep/unix/io.c b/sysdep/unix/io.c index f6cc0e32..9d54a2c3 100644 --- a/sysdep/unix/io.c +++ b/sysdep/unix/io.c @@ -1495,7 +1495,9 @@ sk_open_unix(sock *s, char *name) if (fcntl(fd, F_SETFL, O_NONBLOCK) < 0) return -1; - /* Path length checked in test_old_bird() */ + /* Path length checked in test_old_bird() but we may need unix sockets for other reasons in future */ + ASSERT_DIE(strlen(name) < sizeof(sa.sun_path)); + sa.sun_family = AF_UNIX; strcpy(sa.sun_path, name); diff --git a/sysdep/unix/krt.c b/sysdep/unix/krt.c index 42dd12f6..cccee456 100644 --- a/sysdep/unix/krt.c +++ b/sysdep/unix/krt.c @@ -1156,7 +1156,7 @@ krt_copy_config(struct proto_config *dest, struct proto_config *src) } static int -krt_get_attr(eattr *a, byte *buf, int buflen) +krt_get_attr(const eattr *a, byte *buf, int buflen) { switch (a->id) { diff --git a/sysdep/unix/krt.h b/sysdep/unix/krt.h index 6066f2f1..4a5d10d2 100644 --- a/sysdep/unix/krt.h +++ b/sysdep/unix/krt.h @@ -141,7 +141,7 @@ void krt_sys_copy_config(struct krt_config *, struct krt_config *); int krt_capable(rte *e); void krt_do_scan(struct krt_proto *); void krt_replace_rte(struct krt_proto *p, net *n, rte *new, rte *old); -int krt_sys_get_attr(eattr *a, byte *buf, int buflen); +int krt_sys_get_attr(const eattr *a, byte *buf, int buflen); /* kif sysdep */ diff --git a/test/birdtest.c b/test/birdtest.c index a092446a..641fd3c7 100644 --- a/test/birdtest.c +++ b/test/birdtest.c @@ -495,7 +495,10 @@ void bt_fmt_ipa(char *buf, size_t size, const void *data) { const ip_addr *ip = data; - bsnprintf(buf, size, "%I", *ip); + if (data) + bsnprintf(buf, size, "%I", *ip); + else + bsnprintf(buf, size, "(null)"); } int