From 2a022368f4b37559de5d5621a88c648023493f75 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Thu, 2 Sep 2010 10:09:09 -0700 Subject: [PATCH 1/1] Avoid shadowing local variable names. All of these changes avoid using the same name for two local variables within a same function. None of them are actual bugs as far as I can tell, but any of them could be confusing to the casual reader. The one in lib/ovsdb-idl.c is particularly brilliant: inner and outer loops both using (different) variables named 'i'. Found with GCC -Wshadow. --- lib/dpif-netdev.c | 1 - lib/dynamic-string.c | 2 -- lib/json.c | 1 - lib/learning-switch.c | 6 +++--- lib/netdev-linux.c | 6 +++--- lib/netlink.c | 10 +++++----- lib/ofp-parse.c | 6 +++--- lib/ovsdb-idl.c | 8 ++++---- lib/process.c | 2 +- lib/stream-fd.c | 2 +- lib/stream-ssl.c | 6 +++--- ofproto/ofproto.c | 2 -- ovsdb/execution.c | 2 -- tests/test-csum.c | 3 +-- tests/test-ovsdb.c | 6 ++++-- utilities/ovs-controller.c | 6 ++---- utilities/ovs-openflowd.c | 4 ---- utilities/ovs-vsctl.c | 24 +++++++++++------------- vswitchd/bridge.c | 4 ---- 19 files changed, 41 insertions(+), 60 deletions(-) diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 323f36411..3975b5a8b 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -1104,7 +1104,6 @@ dp_netdev_modify_vlan_tci(struct ofpbuf *packet, uint16_t tci, uint16_t mask) veh->veth_tci |= htons(tci); } else { /* Insert new 802.1Q header. */ - struct eth_header *eh = packet->l2; struct vlan_eth_header tmp; memcpy(tmp.veth_dst, eh->eth_dst, ETH_ADDR_LEN); memcpy(tmp.veth_src, eh->eth_src, ETH_ADDR_LEN); diff --git a/lib/dynamic-string.c b/lib/dynamic-string.c index 5f8054a45..3af7fc9f5 100644 --- a/lib/dynamic-string.c +++ b/lib/dynamic-string.c @@ -147,8 +147,6 @@ ds_put_format_valist(struct ds *ds, const char *format, va_list args_) if (needed < available) { ds->length += needed; } else { - size_t available; - ds_reserve(ds, ds->length + needed); va_copy(args, args_); diff --git a/lib/json.c b/lib/json.c index 3b70e6bdb..5887f677a 100644 --- a/lib/json.c +++ b/lib/json.c @@ -705,7 +705,6 @@ json_lex_number(struct json_parser *p) * * We suppress negative zeros as a matter of policy. */ if (!significand) { - struct json_token token; token.type = T_INTEGER; token.u.integer = 0; json_parser_input(p, &token); diff --git a/lib/learning-switch.c b/lib/learning-switch.c index e189f1e44..4e7645d7c 100644 --- a/lib/learning-switch.c +++ b/lib/learning-switch.c @@ -220,10 +220,10 @@ lswitch_process_packet(struct lswitch *sw, struct rconn *rconn, } } if (VLOG_IS_DBG_ENABLED()) { - char *p = ofp_to_string(msg->data, msg->size, 2); + char *s = ofp_to_string(msg->data, msg->size, 2); VLOG_DBG_RL(&rl, "%016llx: OpenFlow packet ignored: %s", - sw->datapath_id, p); - free(p); + sw->datapath_id, s); + free(s); } } diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index e6036bfc5..7227f5dfc 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -1779,12 +1779,12 @@ netdev_linux_get_in6(const struct netdev *netdev_, struct in6_addr *in6) if (file != NULL) { const char *name = netdev_get_name(netdev_); while (fgets(line, sizeof line, file)) { - struct in6_addr in6; + struct in6_addr in6_tmp; char ifname[16 + 1]; - if (parse_if_inet6_line(line, &in6, ifname) + if (parse_if_inet6_line(line, &in6_tmp, ifname) && !strcmp(name, ifname)) { - netdev_dev->in6 = in6; + netdev_dev->in6 = in6_tmp; break; } } diff --git a/lib/netlink.c b/lib/netlink.c index 4e83747cc..66c27b1fb 100644 --- a/lib/netlink.c +++ b/lib/netlink.c @@ -1036,19 +1036,19 @@ nl_policy_parse(const struct ofpbuf *msg, size_t nla_offset, type = nla->nla_type; if (type < n_attrs && policy[type].type != NL_A_NO_ATTR) { - const struct nl_policy *p = &policy[type]; + const struct nl_policy *e = &policy[type]; size_t min_len, max_len; /* Validate length and content. */ - min_len = p->min_len ? p->min_len : attr_len_range[p->type][0]; - max_len = p->max_len ? p->max_len : attr_len_range[p->type][1]; + min_len = e->min_len ? e->min_len : attr_len_range[e->type][0]; + max_len = e->max_len ? e->max_len : attr_len_range[e->type][1]; if (len < min_len || len > max_len) { VLOG_DBG_RL(&rl, "%zu: attr %"PRIu16" length %zu not in " "allowed range %zu...%zu", offset, type, len, min_len, max_len); return false; } - if (p->type == NL_A_STRING) { + if (e->type == NL_A_STRING) { if (((char *) nla)[nla->nla_len - 1]) { VLOG_DBG_RL(&rl, "%zu: attr %"PRIu16" lacks null at end", offset, type); @@ -1060,7 +1060,7 @@ nl_policy_parse(const struct ofpbuf *msg, size_t nla_offset, return false; } } - if (!p->optional && attrs[type] == NULL) { + if (!e->optional && attrs[type] == NULL) { assert(n_required > 0); --n_required; } diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c index 405008c67..312eaaaaf 100644 --- a/lib/ofp-parse.c +++ b/lib/ofp-parse.c @@ -270,12 +270,12 @@ str_to_action(char *str, struct ofpbuf *b) put_output_action(b, str_to_u32(arg)); } else if (!strcasecmp(act, "enqueue")) { char *sp = NULL; - char *port = strtok_r(arg, ":q", &sp); + char *port_s = strtok_r(arg, ":q", &sp); char *queue = strtok_r(NULL, "", &sp); - if (port == NULL || queue == NULL) { + if (port_s == NULL || queue == NULL) { ovs_fatal(0, "\"enqueue\" syntax is \"enqueue:PORT:QUEUE\""); } - put_enqueue_action(b, str_to_u32(port), str_to_u32(queue)); + put_enqueue_action(b, str_to_u32(port_s), str_to_u32(queue)); } else if (!strcasecmp(act, "drop")) { /* A drop action in OpenFlow occurs by just not setting * an action. */ diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c index 2132f9fef..43ff94714 100644 --- a/lib/ovsdb-idl.c +++ b/lib/ovsdb-idl.c @@ -433,13 +433,13 @@ ovsdb_idl_send_monitor_request(struct ovsdb_idl *idl) const struct ovsdb_idl_table *table = &idl->tables[i]; const struct ovsdb_idl_table_class *tc = table->class; struct json *monitor_request, *columns; - size_t i; + size_t j; monitor_request = json_object_create(); columns = json_array_create_empty(); - for (i = 0; i < tc->n_columns; i++) { - const struct ovsdb_idl_column *column = &tc->columns[i]; - if (table->modes[i] != OVSDB_IDL_MODE_NONE) { + for (j = 0; j < tc->n_columns; j++) { + const struct ovsdb_idl_column *column = &tc->columns[j]; + if (table->modes[j] != OVSDB_IDL_MODE_NONE) { json_array_add(columns, json_string_create(column->name)); } } diff --git a/lib/process.c b/lib/process.c index a201a88f8..377c396b9 100644 --- a/lib/process.c +++ b/lib/process.c @@ -517,7 +517,7 @@ process_run_capture(char **argv, char **stdout_log, char **stderr_log, block_sigchld(&oldsigs); pid = fork(); if (pid < 0) { - int error = errno; + error = errno; unblock_sigchld(&oldsigs); VLOG_WARN("fork failed: %s", strerror(error)); diff --git a/lib/stream-fd.c b/lib/stream-fd.c index 9410009c4..ef4dc8d91 100644 --- a/lib/stream-fd.c +++ b/lib/stream-fd.c @@ -214,7 +214,7 @@ pfd_accept(struct pstream *pstream, struct stream **new_streamp) new_fd = accept(ps->fd, (struct sockaddr *) &ss, &ss_len); if (new_fd < 0) { - int retval = errno; + retval = errno; if (retval != EAGAIN) { VLOG_DBG_RL(&rl, "accept: %s", strerror(retval)); } diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c index 70b15f0da..9c7533d1e 100644 --- a/lib/stream-ssl.c +++ b/lib/stream-ssl.c @@ -385,7 +385,7 @@ do_ca_cert_bootstrap(struct stream *stream) file = fdopen(fd, "w"); if (!file) { - int error = errno; + error = errno; VLOG_ERR("could not bootstrap CA cert: fdopen failed: %s", strerror(error)); unlink(ca_cert.file_name); @@ -402,7 +402,7 @@ do_ca_cert_bootstrap(struct stream *stream) } if (fclose(file)) { - int error = errno; + error = errno; VLOG_ERR("could not bootstrap CA cert: writing %s failed: %s", ca_cert.file_name, strerror(error)); unlink(ca_cert.file_name); @@ -921,7 +921,7 @@ pssl_accept(struct pstream *pstream, struct stream **new_streamp) new_fd = accept(pssl->fd, &sin, &sin_len); if (new_fd < 0) { - int error = errno; + error = errno; if (error != EAGAIN) { VLOG_DBG_RL(&rl, "accept: %s", strerror(error)); } diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 844083d8b..e571bd4e2 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -1071,7 +1071,6 @@ ofproto_run1(struct ofproto *p) for (i = 0; i < 50; i++) { struct ofpbuf *buf; - int error; error = dpif_recv(p->dpif, &buf); if (error) { @@ -1122,7 +1121,6 @@ ofproto_run1(struct ofproto *p) retval = pvconn_accept(ofservice->pvconn, OFP_VERSION, &vconn); if (!retval) { - struct ofconn *ofconn; struct rconn *rconn; char *name; diff --git a/ovsdb/execution.c b/ovsdb/execution.c index 7ce9a3f50..a96abfcaf 100644 --- a/ovsdb/execution.c +++ b/ovsdb/execution.c @@ -103,8 +103,6 @@ ovsdb_execute(struct ovsdb *db, const struct json *params, || !params->u.array.n || params->u.array.elems[0]->type != JSON_STRING || strcmp(params->u.array.elems[0]->u.string, db->schema->name)) { - struct ovsdb_error *error; - if (params->type != JSON_ARRAY) { error = ovsdb_syntax_error(params, NULL, "array expected"); } else { diff --git a/tests/test-csum.c b/tests/test-csum.c index 8c8545870..eebc8803f 100644 --- a/tests/test-csum.c +++ b/tests/test-csum.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2009 Nicira Networks. + * Copyright (c) 2009, 2010 Nicira Networks. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -134,7 +134,6 @@ main(void) const uint16_t *data16 = (const uint16_t *) tc->data; const uint32_t *data32 = (const uint32_t *) tc->data; uint32_t partial; - size_t i; /* Test csum(). */ assert(ntohs(csum(tc->data, tc->size)) == tc->csum); diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c index 18784a52a..04db65421 100644 --- a/tests/test-ovsdb.c +++ b/tests/test-ovsdb.c @@ -1095,7 +1095,7 @@ do_query_distinct(int argc OVS_UNUSED, char *argv[]) size_t n_classes; struct json *json; int exit_code = 0; - size_t i, j, k; + size_t i; /* Parse table schema, create table. */ json = unbox_json(parse_json(argv[1])); @@ -1161,6 +1161,7 @@ do_query_distinct(int argc OVS_UNUSED, char *argv[]) for (i = 0; i < json->u.array.n; i++) { struct ovsdb_row_set results; struct ovsdb_condition cnd; + size_t j; check_ovsdb_error(ovsdb_condition_from_json(ts, json->u.array.elems[i], NULL, &cnd)); @@ -1171,6 +1172,8 @@ do_query_distinct(int argc OVS_UNUSED, char *argv[]) ovsdb_row_set_init(&results); ovsdb_query_distinct(table, &cnd, &columns, &results); for (j = 0; j < results.n_rows; j++) { + size_t k; + for (k = 0; k < n_rows; k++) { if (uuid_equals(ovsdb_row_get_uuid(results.rows[j]), &rows[k].uuid)) { @@ -1833,7 +1836,6 @@ do_idl(int argc, char *argv[]) for (i = 2; i < argc; i++) { char *arg = argv[i]; struct jsonrpc_msg *request, *reply; - int error; if (*arg == '+') { /* The previous transaction didn't change anything. */ diff --git a/utilities/ovs-controller.c b/utilities/ovs-controller.c index b18959ad6..40e2a801d 100644 --- a/utilities/ovs-controller.c +++ b/utilities/ovs-controller.c @@ -107,7 +107,6 @@ main(int argc, char *argv[]) for (i = optind; i < argc; i++) { const char *name = argv[i]; struct vconn *vconn; - int retval; retval = vconn_open(name, OFP_VERSION, &vconn); if (!retval) { @@ -146,12 +145,10 @@ main(int argc, char *argv[]) while (n_switches > 0 || n_listeners > 0) { int iteration; - int i; /* Accept connections on listening vconns. */ for (i = 0; i < n_listeners && n_switches < MAX_SWITCHES; ) { struct vconn *new_vconn; - int retval; retval = pvconn_accept(listeners[i], OFP_VERSION, &new_vconn); if (!retval || retval == EAGAIN) { @@ -171,7 +168,8 @@ main(int argc, char *argv[]) bool progress = false; for (i = 0; i < n_switches; ) { struct switch_ *this = &switches[i]; - int retval = do_switching(this); + + retval = do_switching(this); if (!retval || retval == EAGAIN) { if (!retval) { progress = true; diff --git a/utilities/ovs-openflowd.c b/utilities/ovs-openflowd.c index 8cb50e4b4..945b11d05 100644 --- a/utilities/ovs-openflowd.c +++ b/utilities/ovs-openflowd.c @@ -458,8 +458,6 @@ parse_options(int argc, char *argv[], struct ofsettings *s) s->n_controllers = controllers.n; s->controllers = xmalloc(s->n_controllers * sizeof *s->controllers); if (argc > 1) { - size_t i; - for (i = 0; i < s->n_controllers; i++) { s->controllers[i] = controller_opts; s->controllers[i].target = controllers.names[i]; @@ -468,8 +466,6 @@ parse_options(int argc, char *argv[], struct ofsettings *s) /* Sanity check. */ if (controller_opts.band == OFPROTO_OUT_OF_BAND) { - size_t i; - for (i = 0; i < s->n_controllers; i++) { if (!strcmp(s->controllers[i].target, "discover")) { ovs_fatal(0, "Cannot perform discovery with out-of-band " diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c index 043530280..884a41faf 100644 --- a/utilities/ovs-vsctl.c +++ b/utilities/ovs-vsctl.c @@ -1305,12 +1305,11 @@ add_port(struct vsctl_context *ctx, get_info(ctx->ovs, &info); if (may_exist) { - struct vsctl_port *port; + struct vsctl_port *vsctl_port; - port = find_port(&info, port_name, false); - if (port) { + vsctl_port = find_port(&info, port_name, false); + if (vsctl_port) { struct svec want_names, have_names; - size_t i; svec_init(&want_names); for (i = 0; i < n_ifaces; i++) { @@ -1319,15 +1318,16 @@ add_port(struct vsctl_context *ctx, svec_sort(&want_names); svec_init(&have_names); - for (i = 0; i < port->port_cfg->n_interfaces; i++) { - svec_add(&have_names, port->port_cfg->interfaces[i]->name); + for (i = 0; i < vsctl_port->port_cfg->n_interfaces; i++) { + svec_add(&have_names, + vsctl_port->port_cfg->interfaces[i]->name); } svec_sort(&have_names); - if (strcmp(port->bridge->name, br_name)) { + if (strcmp(vsctl_port->bridge->name, br_name)) { char *command = vsctl_context_to_string(ctx); vsctl_fatal("\"%s\" but %s is actually attached to bridge %s", - command, port_name, port->bridge->name); + command, port_name, vsctl_port->bridge->name); } if (!svec_equal(&want_names, &have_names)) { @@ -2767,8 +2767,8 @@ do_vsctl(const char *args, struct vsctl_command *commands, size_t n_commands, ds_chomp(ds, '\n'); for (j = 0; j < ds->length; j++) { - int c = ds->string[j]; - switch (c) { + int ch = ds->string[j]; + switch (ch) { case '\n': fputs("\\n", stdout); break; @@ -2778,7 +2778,7 @@ do_vsctl(const char *args, struct vsctl_command *commands, size_t n_commands, break; default: - putchar(c); + putchar(ch); } } putchar('\n'); @@ -2796,8 +2796,6 @@ do_vsctl(const char *args, struct vsctl_command *commands, size_t n_commands, if (wait_for_reload && status != TXN_UNCHANGED) { for (;;) { - const struct ovsrec_open_vswitch *ovs; - ovsdb_idl_run(idl); OVSREC_OPEN_VSWITCH_FOR_EACH (ovs, idl) { if (ovs->cur_cfg >= next_cfg) { diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index 598b0016d..3f5e3d471 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -632,7 +632,6 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg) struct odp_port *dpif_ports; size_t n_dpif_ports; struct shash cur_ifaces, want_ifaces; - struct shash_node *node; /* Get the set of interfaces currently in this datapath. */ dpif_port_list(br->dpif, &dpif_ports, &n_dpif_ports); @@ -765,7 +764,6 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg) struct ovsrec_controller **controllers; struct ofproto_sflow_options oso; size_t n_controllers; - size_t i; memset(&oso, 0, sizeof oso); @@ -2822,7 +2820,6 @@ bond_rebalance_port(struct port *port) * smallest hashes instead of the biggest ones. There is little * reason behind this decision; we could use the opposite sort * order to shift away big hashes ahead of small ones. */ - size_t i; bool order_swapped; for (i = 0; i < from->n_hashes; i++) { @@ -3407,7 +3404,6 @@ port_reconfigure(struct port *port, const struct ovsrec_port *cfg) trunks = NULL; if (vlan < 0 && cfg->n_trunks) { size_t n_errors; - size_t i; trunks = bitmap_allocate(4096); n_errors = 0; -- 2.43.0