[ovs-dev] [RFC] treewide: undefined behavior, passing null in nonnull parameters
Lance Richardson
lrichard at redhat.com
Tue Jun 13 00:06:01 UTC 2017
Eliminate a number of instances of undefined behavior related to
passing NULL in parameters having "nonnull" annotations.
Found with gcc's undefined behavior sanitizer.
Signed-off-by: Lance Richardson <lrichard at redhat.com>
---
Posting this as RFC because there is no apparent risk of
unwanted compiler optimizations related to undefined behavior
in existing code. The main value in fixing these issues is
in reducing noise to make it easier to find problematic
cases in the future.
Here is a small example of the type of unwanted optimization
to be concerned about:
test1a.c:
#include <stdio.h>
extern void foo(char*, size_t);
int main(int argc, char **argv)
{
char x[128];
foo(x, sizeof x);
foo(NULL, 0);
return 0;
}
test1b.c:
#include <stdio.h>
#include <string.h>
void foo(char *bar, size_t len)
{
memset(bar, 0, len);
if (bar)
printf("bar is non-null: %p\n", bar);
}
Compile and run:
gcc -o test -O2 test1a.c test1b.c
./test
Output (second line might be a bit of a surprise):
bar is non-null: 0x7fff80f90d50
bar is non-null: (nil)
lib/netlink.c | 7 ++++++-
lib/ofpbuf.c | 4 +++-
lib/svec.c | 4 +++-
lib/util.c | 4 +++-
ovn/utilities/ovn-nbctl.c | 32 ++++++++++++++++++++++++--------
5 files changed, 39 insertions(+), 12 deletions(-)
diff --git a/lib/netlink.c b/lib/netlink.c
index 3da22a1..fcad884 100644
--- a/lib/netlink.c
+++ b/lib/netlink.c
@@ -241,7 +241,12 @@ void
nl_msg_put_unspec(struct ofpbuf *msg, uint16_t type,
const void *data, size_t size)
{
- memcpy(nl_msg_put_unspec_uninit(msg, type, size), data, size);
+ void *ptr;
+
+ ptr = nl_msg_put_unspec_uninit(msg, type, size);
+ if (size) {
+ memcpy(ptr, data, size);
+ }
}
/* Appends a Netlink attribute of the given 'type' and no payload to 'msg'.
diff --git a/lib/ofpbuf.c b/lib/ofpbuf.c
index 3019c4a..2e71fed 100644
--- a/lib/ofpbuf.c
+++ b/lib/ofpbuf.c
@@ -375,7 +375,9 @@ void *
ofpbuf_put_zeros(struct ofpbuf *b, size_t size)
{
void *dst = ofpbuf_put_uninit(b, size);
- memset(dst, 0, size);
+ if (size) {
+ memset(dst, 0, size);
+ }
return dst;
}
diff --git a/lib/svec.c b/lib/svec.c
index aad04e3..297a60c 100644
--- a/lib/svec.c
+++ b/lib/svec.c
@@ -127,7 +127,9 @@ compare_strings(const void *a_, const void *b_)
void
svec_sort(struct svec *svec)
{
- qsort(svec->names, svec->n, sizeof *svec->names, compare_strings);
+ if (svec->n) {
+ qsort(svec->names, svec->n, sizeof *svec->names, compare_strings);
+ }
}
void
diff --git a/lib/util.c b/lib/util.c
index b2a1f8a..ddf8546 100644
--- a/lib/util.c
+++ b/lib/util.c
@@ -132,7 +132,9 @@ void *
xmemdup(const void *p_, size_t size)
{
void *p = xmalloc(size);
- memcpy(p, p_, size);
+ if (size) {
+ memcpy(p, p_, size);
+ }
return p;
}
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index bd0160a..3b2d3e9 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -920,7 +920,9 @@ nbctl_lsp_add(struct ctl_context *ctx)
nbrec_logical_switch_verify_ports(ls);
struct nbrec_logical_switch_port **new_ports = xmalloc(sizeof *new_ports *
(ls->n_ports + 1));
- memcpy(new_ports, ls->ports, sizeof *new_ports * ls->n_ports);
+ if (ls->n_ports) {
+ memcpy(new_ports, ls->ports, sizeof *new_ports * ls->n_ports);
+ }
new_ports[ls->n_ports] = CONST_CAST(struct nbrec_logical_switch_port *,
lsp);
nbrec_logical_switch_set_ports(ls, new_ports, ls->n_ports + 1);
@@ -1379,7 +1381,9 @@ nbctl_acl_add(struct ctl_context *ctx)
/* Insert the acl into the logical switch. */
nbrec_logical_switch_verify_acls(ls);
struct nbrec_acl **new_acls = xmalloc(sizeof *new_acls * (ls->n_acls + 1));
- memcpy(new_acls, ls->acls, sizeof *new_acls * ls->n_acls);
+ if (ls->n_acls) {
+ memcpy(new_acls, ls->acls, sizeof *new_acls * ls->n_acls);
+ }
new_acls[ls->n_acls] = acl;
nbrec_logical_switch_set_acls(ls, new_acls, ls->n_acls + 1);
free(new_acls);
@@ -1697,7 +1701,10 @@ nbctl_lr_lb_add(struct ctl_context *ctx)
struct nbrec_load_balancer **new_lbs
= xmalloc(sizeof *new_lbs * (lr->n_load_balancer + 1));
- memcpy(new_lbs, lr->load_balancer, sizeof *new_lbs * lr->n_load_balancer);
+ if (lr->n_load_balancer) {
+ memcpy(new_lbs, lr->load_balancer,
+ sizeof *new_lbs * lr->n_load_balancer);
+ }
new_lbs[lr->n_load_balancer] = CONST_CAST(struct nbrec_load_balancer *,
new_lb);
nbrec_logical_router_set_load_balancer(lr, new_lbs,
@@ -1793,7 +1800,10 @@ nbctl_ls_lb_add(struct ctl_context *ctx)
struct nbrec_load_balancer **new_lbs
= xmalloc(sizeof *new_lbs * (ls->n_load_balancer + 1));
- memcpy(new_lbs, ls->load_balancer, sizeof *new_lbs * ls->n_load_balancer);
+ if (ls->n_load_balancer) {
+ memcpy(new_lbs, ls->load_balancer,
+ sizeof *new_lbs * ls->n_load_balancer);
+ }
new_lbs[ls->n_load_balancer] = CONST_CAST(struct nbrec_load_balancer *,
new_lb);
nbrec_logical_switch_set_load_balancer(ls, new_lbs,
@@ -2200,8 +2210,10 @@ nbctl_lr_route_add(struct ctl_context *ctx)
nbrec_logical_router_verify_static_routes(lr);
struct nbrec_logical_router_static_route **new_routes
= xmalloc(sizeof *new_routes * (lr->n_static_routes + 1));
- memcpy(new_routes, lr->static_routes,
- sizeof *new_routes * lr->n_static_routes);
+ if (lr->n_static_routes) {
+ memcpy(new_routes, lr->static_routes,
+ sizeof *new_routes * lr->n_static_routes);
+ }
new_routes[lr->n_static_routes] = route;
nbrec_logical_router_set_static_routes(lr, new_routes,
lr->n_static_routes + 1);
@@ -2364,7 +2376,9 @@ nbctl_lr_nat_add(struct ctl_context *ctx)
/* Insert the NAT into the logical router. */
nbrec_logical_router_verify_nat(lr);
struct nbrec_nat **new_nats = xmalloc(sizeof *new_nats * (lr->n_nat + 1));
- memcpy(new_nats, lr->nat, sizeof *new_nats * lr->n_nat);
+ if (lr->n_nat) {
+ memcpy(new_nats, lr->nat, sizeof *new_nats * lr->n_nat);
+ }
new_nats[lr->n_nat] = nat;
nbrec_logical_router_set_nat(lr, new_nats, lr->n_nat + 1);
free(new_nats);
@@ -2642,7 +2656,9 @@ nbctl_lrp_add(struct ctl_context *ctx)
nbrec_logical_router_verify_ports(lr);
struct nbrec_logical_router_port **new_ports = xmalloc(sizeof *new_ports *
(lr->n_ports + 1));
- memcpy(new_ports, lr->ports, sizeof *new_ports * lr->n_ports);
+ if (lr->n_ports) {
+ memcpy(new_ports, lr->ports, sizeof *new_ports * lr->n_ports);
+ }
new_ports[lr->n_ports] = CONST_CAST(struct nbrec_logical_router_port *,
lrp);
nbrec_logical_router_set_ports(lr, new_ports, lr->n_ports + 1);
--
2.9.4
More information about the dev
mailing list