Skip to content

Commit

Permalink
Eliminate most shadowing for local variable names.
Browse files Browse the repository at this point in the history
Shadowing is when a variable with a given name in an inner scope hides a
different variable with the same name in a surrounding scope.  This is
generally undesirable because it can confuse programmers.  This commit
eliminates most of it.

Found with -Wshadow=local in GCC 7.  The repo is not really ready to enable
this option by default because of a few cases that are harder to fix, and
harmless, such as nested use of CMAP_FOR_EACH.

Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Andy Zhou <azhou@ovn.org>
  • Loading branch information
blp committed Aug 2, 2017
1 parent b24fa3f commit 71f2127
Show file tree
Hide file tree
Showing 37 changed files with 128 additions and 166 deletions.
2 changes: 0 additions & 2 deletions lib/classifier.c
Expand Up @@ -793,8 +793,6 @@ classifier_remove(struct classifier *cls, const struct cls_rule *cls_rule)
&& --subtable->max_count == 0) {
/* Find the new 'max_priority' and 'max_count'. */
int max_priority = INT_MIN;
struct cls_match *head;

CMAP_FOR_EACH (head, cmap_node, &subtable->rules) {
if (head->priority > max_priority) {
max_priority = head->priority;
Expand Down
6 changes: 3 additions & 3 deletions lib/daemon-unix.c
Expand Up @@ -1002,11 +1002,11 @@ daemon_set_new_user(const char *user_spec)
grpstr += strspn(grpstr, " \t\r\n");

if (*grpstr) {
struct group grp, *res;
struct group grp, *gres;

bufsize = init_bufsize;
buf = xmalloc(bufsize);
while ((e = getgrnam_r(grpstr, &grp, buf, bufsize, &res))
while ((e = getgrnam_r(grpstr, &grp, buf, bufsize, &gres))
== ERANGE) {
if (!enlarge_buffer(&buf, &bufsize)) {
break;
Expand All @@ -1018,7 +1018,7 @@ daemon_set_new_user(const char *user_spec)
"(%s), aborting.", pidfile, grpstr,
ovs_strerror(e));
}
if (res == NULL) {
if (gres == NULL) {
VLOG_FATAL("%s: group %s not found, aborting.", pidfile,
grpstr);
}
Expand Down
4 changes: 2 additions & 2 deletions lib/db-ctl-base.c
Expand Up @@ -313,9 +313,9 @@ get_row_by_id(struct ctl_context *ctx,
if (!id->key) {
name = datum->n == 1 ? &datum->keys[0] : NULL;
} else {
const union ovsdb_atom key
const union ovsdb_atom key_atom
= { .string = CONST_CAST(char *, id->key) };
unsigned int i = ovsdb_datum_find_key(datum, &key,
unsigned int i = ovsdb_datum_find_key(datum, &key_atom,
OVSDB_TYPE_STRING);
name = i == UINT_MAX ? NULL : &datum->values[i];
}
Expand Down
6 changes: 3 additions & 3 deletions lib/dpctl.c
Expand Up @@ -336,7 +336,8 @@ dpctl_set_if(int argc, const char *argv[], struct dpctl_params *dpctl_p)
struct smap args;
odp_port_t port_no;
char *option;
int error = 0;

error = 0;

argcopy = xstrdup(argv[i]);
name = strtok_r(argcopy, ",", &save_ptr);
Expand Down Expand Up @@ -1787,8 +1788,7 @@ dpctl_normalize_actions(int argc, const char *argv[],
qsort(afs, n_afs, sizeof *afs, compare_actions_for_flow);

for (i = 0; i < n_afs; i++) {
struct actions_for_flow *af = afs[i];

af = afs[i];
sort_output_actions(af->actions.data, af->actions.size);

for (encaps = 0; encaps < FLOW_MAX_VLAN_HEADERS; encaps ++) {
Expand Down
9 changes: 4 additions & 5 deletions lib/dpif-netdev.c
Expand Up @@ -1016,14 +1016,13 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int argc, const char *argv[],
} else {
unsigned long long stats[DP_N_STATS];
uint64_t cycles[PMD_N_CYCLES];
int i;

/* Read current stats and cycle counters */
for (i = 0; i < ARRAY_SIZE(stats); i++) {
atomic_read_relaxed(&pmd->stats.n[i], &stats[i]);
for (size_t j = 0; j < ARRAY_SIZE(stats); j++) {
atomic_read_relaxed(&pmd->stats.n[j], &stats[j]);
}
for (i = 0; i < ARRAY_SIZE(cycles); i++) {
atomic_read_relaxed(&pmd->cycles.n[i], &cycles[i]);
for (size_t j = 0; j < ARRAY_SIZE(cycles); j++) {
atomic_read_relaxed(&pmd->cycles.n[j], &cycles[j]);
}

if (type == PMD_INFO_CLEAR_STATS) {
Expand Down
2 changes: 1 addition & 1 deletion lib/hash.c
Expand Up @@ -87,7 +87,6 @@ hash_bytes128(const void *p_, size_t len, uint32_t basis, ovs_u128 *out)
uint32_t h2 = basis;
uint32_t h3 = basis;
uint32_t h4 = basis;
uint32_t k1, k2, k3, k4;

/* Body */
for (int i = 0; i < nblocks; i++) {
Expand Down Expand Up @@ -134,6 +133,7 @@ hash_bytes128(const void *p_, size_t len, uint32_t basis, ovs_u128 *out)
}

/* Tail */
uint32_t k1, k2, k3, k4;
k1 = k2 = k3 = k4 = 0;
tail = data + nblocks * 16;
switch (len & 15) {
Expand Down
7 changes: 3 additions & 4 deletions lib/learn.c
Expand Up @@ -342,14 +342,13 @@ learn_parse_spec(const char *orig, char *name, char *value,
name, value);
}

char *error = learn_parse_load_immediate(&imm, dst_value + 2, value, spec,
ofpacts);
error = learn_parse_load_immediate(&imm, dst_value + 2, value, spec,
ofpacts);
if (error) {
return error;
}
} else {
struct ofpact_reg_move move;
char *error;

error = nxm_parse_reg_move(&move, value);
if (error) {
Expand All @@ -363,7 +362,7 @@ learn_parse_spec(const char *orig, char *name, char *value,
spec->dst = move.dst;
}
} else if (!strcmp(name, "output")) {
char *error = mf_parse_subfield(&spec->src, value);
error = mf_parse_subfield(&spec->src, value);
if (error) {
return error;
}
Expand Down
2 changes: 1 addition & 1 deletion lib/lldp/lldpd.c
Expand Up @@ -339,7 +339,7 @@ lldpd_decode(struct lldpd *cfg, char *frame, int s,

/* No, but do we already know the system? */
if (!oport) {
bool found = false;
found = false;
VLOG_DBG("MSAP is unknown, search for the chassis");

LIST_FOR_EACH (ochassis, list, &cfg->g_chassis) {
Expand Down
12 changes: 6 additions & 6 deletions lib/netdev-dummy.c
Expand Up @@ -1102,11 +1102,11 @@ netdev_dummy_send(struct netdev *netdev, int qid OVS_UNUSED,

/* Reply to ARP requests for 'dev''s assigned IP address. */
if (dev->address.s_addr) {
struct dp_packet packet;
struct dp_packet dp;
struct flow flow;

dp_packet_use_const(&packet, buffer, size);
flow_extract(&packet, &flow);
dp_packet_use_const(&dp, buffer, size);
flow_extract(&dp, &flow);
if (flow.dl_type == htons(ETH_TYPE_ARP)
&& flow.nw_proto == ARP_OP_REQUEST
&& flow.nw_dst == dev->address.s_addr) {
Expand All @@ -1118,10 +1118,10 @@ netdev_dummy_send(struct netdev *netdev, int qid OVS_UNUSED,
}

if (dev->tx_pcap) {
struct dp_packet packet;
struct dp_packet dp;

dp_packet_use_const(&packet, buffer, size);
ovs_pcap_write(dev->tx_pcap, &packet);
dp_packet_use_const(&dp, buffer, size);
ovs_pcap_write(dev->tx_pcap, &dp);
fflush(dev->tx_pcap);
}

Expand Down
2 changes: 1 addition & 1 deletion lib/netdev-linux.c
Expand Up @@ -5373,7 +5373,7 @@ get_stats_via_netlink(const struct netdev *netdev_, struct netdev_stats *stats)
netdev_stats_from_rtnl_link_stats64(stats, nl_attr_get(a));
error = 0;
} else {
const struct nlattr *a = nl_attr_find(reply, 0, IFLA_STATS);
a = nl_attr_find(reply, 0, IFLA_STATS);
if (a && nl_attr_get_size(a) >= sizeof(struct rtnl_link_stats)) {
netdev_stats_from_rtnl_link_stats(stats, nl_attr_get(a));
error = 0;
Expand Down
10 changes: 4 additions & 6 deletions lib/odp-util.c
Expand Up @@ -4264,13 +4264,11 @@ static int
parse_odp_key_mask_attr(const char *s, const struct simap *port_names,
struct ofpbuf *key, struct ofpbuf *mask)
{
ovs_u128 ufid;
int len;

/* Skip UFID. */
len = odp_ufid_from_string(s, &ufid);
if (len) {
return len;
ovs_u128 ufid;
int ufid_len = odp_ufid_from_string(s, &ufid);
if (ufid_len) {
return ufid_len;
}

SCAN_SINGLE("skb_priority(", uint32_t, u32, OVS_KEY_ATTR_PRIORITY);
Expand Down
4 changes: 2 additions & 2 deletions lib/ofp-print.c
Expand Up @@ -2344,12 +2344,12 @@ ofp_async_config_reason_to_string(uint32_t reason,
#define OFP_ASYNC_CONFIG_REASON_BUFSIZE (INT_STRLEN(int) + 1)
static void
ofp_print_set_async_config(struct ds *string, const struct ofp_header *oh,
enum ofptype type)
enum ofptype ofptype)
{
struct ofputil_async_cfg basis = OFPUTIL_ASYNC_CFG_INIT;
struct ofputil_async_cfg ac;

bool is_reply = type == OFPTYPE_GET_ASYNC_REPLY;
bool is_reply = ofptype == OFPTYPE_GET_ASYNC_REPLY;
enum ofperr error = ofputil_decode_set_async_config(oh, is_reply,
&basis, &ac);
if (error) {
Expand Down
1 change: 0 additions & 1 deletion lib/ofp-util.c
Expand Up @@ -8314,7 +8314,6 @@ ofputil_pull_ofp14_port_stats(struct ofputil_port_stats *ops,
len);
while (properties.size > 0) {
struct ofpbuf payload;
enum ofperr error;
uint64_t type = 0;

error = ofpprop_pull(&properties, &payload, &type);
Expand Down
4 changes: 2 additions & 2 deletions lib/ovsdb-idl.c
Expand Up @@ -787,10 +787,10 @@ ovsdb_idl_check_consistency(const struct ovsdb_idl *idl)
ok = false;
}
}
for (size_t i = 0; i < n_dsts; i++) {
for (size_t j = 0; j < n_dsts; j++) {
VLOG_ERR("%s row "UUID_FMT" missing arc to row "UUID_FMT,
table->class->name, UUID_ARGS(&row->uuid),
UUID_ARGS(&dsts[i]));
UUID_ARGS(&dsts[j]));
ok = false;
}
}
Expand Down
3 changes: 1 addition & 2 deletions lib/table.c
Expand Up @@ -490,7 +490,6 @@ table_print_json__(const struct table *table, const struct table_style *style)
{
struct json *json, *headings, *data;
size_t x, y;
char *s;

json = json_object_create();
if (table->caption) {
Expand Down Expand Up @@ -526,7 +525,7 @@ table_print_json__(const struct table *table, const struct table_style *style)
}
json_object_put(json, "data", data);

s = json_to_string(json, style->json_flags);
char *s = json_to_string(json, style->json_flags);
json_destroy(json);
puts(s);
free(s);
Expand Down
6 changes: 2 additions & 4 deletions lib/unixctl.c
Expand Up @@ -371,14 +371,11 @@ kill_connection(struct unixctl_conn *conn)
void
unixctl_server_run(struct unixctl_server *server)
{
struct unixctl_conn *conn, *next;
int i;

if (!server) {
return;
}

for (i = 0; i < 10; i++) {
for (int i = 0; i < 10; i++) {
struct stream *stream;
int error;

Expand All @@ -396,6 +393,7 @@ unixctl_server_run(struct unixctl_server *server)
}
}

struct unixctl_conn *conn, *next;
LIST_FOR_EACH_SAFE (conn, next, node, &server->conns) {
int error = run_connection(conn);
if (error && error != EAGAIN) {
Expand Down
1 change: 0 additions & 1 deletion lib/vconn.c
Expand Up @@ -877,7 +877,6 @@ vconn_transact_noreply(struct vconn *vconn, struct ofpbuf *request,
for (;;) {
struct ofpbuf *msg;
ovs_be32 msg_xid;
int error;

error = vconn_recv_block(vconn, &msg);
if (error) {
Expand Down
3 changes: 2 additions & 1 deletion lib/vlog.c
Expand Up @@ -843,7 +843,6 @@ vlog_get_levels(void)
struct ds s = DS_EMPTY_INITIALIZER;
struct vlog_module *mp;
struct svec lines = SVEC_EMPTY_INITIALIZER;
char *line;
size_t i;

ds_put_format(&s, " console syslog file\n");
Expand All @@ -869,6 +868,8 @@ vlog_get_levels(void)
ovs_mutex_unlock(&log_file_mutex);

svec_sort(&lines);

char *line;
SVEC_FOR_EACH (i, line, &lines) {
ds_put_cstr(&s, line);
}
Expand Down
15 changes: 8 additions & 7 deletions ofproto/ofproto-dpif-xlate.c
Expand Up @@ -1901,14 +1901,14 @@ mirror_packet(struct xlate_ctx *ctx, struct xbundle *xbundle,
}
} else if (xvlan.v[0].vid != out_vlan
&& !eth_addr_is_reserved(ctx->xin->flow.dl_dst)) {
struct xbundle *xbundle;
struct xbundle *xb;
uint16_t old_vid = xvlan.v[0].vid;

xvlan.v[0].vid = out_vlan;
LIST_FOR_EACH (xbundle, list_node, &xbridge->xbundles) {
if (xbundle_includes_vlan(xbundle, &xvlan)
&& !xbundle_mirror_out(xbridge, xbundle)) {
output_normal(ctx, xbundle, &xvlan);
LIST_FOR_EACH (xb, list_node, &xbridge->xbundles) {
if (xbundle_includes_vlan(xb, &xvlan)
&& !xbundle_mirror_out(xbridge, xb)) {
output_normal(ctx, xb, &xvlan);
}
}
xvlan.v[0].vid = old_vid;
Expand Down Expand Up @@ -7068,8 +7068,9 @@ xlate_send_packet(const struct ofport_dpif *ofport, bool oam,
}

if (oam) {
const ovs_be16 oam = htons(NX_TUN_FLAG_OAM);
ofpact_put_set_field(&ofpacts, mf_from_id(MFF_TUN_FLAGS), &oam, &oam);
const ovs_be16 flag = htons(NX_TUN_FLAG_OAM);
ofpact_put_set_field(&ofpacts, mf_from_id(MFF_TUN_FLAGS),
&flag, &flag);
}

ofpact_put_OUTPUT(&ofpacts)->port = xport->ofp_port;
Expand Down
13 changes: 4 additions & 9 deletions ofproto/ofproto-dpif.c
Expand Up @@ -4722,7 +4722,6 @@ group_dpif_credit_stats(struct group_dpif *group,
bucket->stats.packet_count += stats->n_packets;
bucket->stats.byte_count += stats->n_bytes;
} else { /* Credit to all buckets */
struct ofputil_bucket *bucket;
LIST_FOR_EACH (bucket, list_node, &group->up.buckets) {
bucket->stats.packet_count += stats->n_packets;
bucket->stats.byte_count += stats->n_bytes;
Expand Down Expand Up @@ -5203,14 +5202,10 @@ dpif_show_backer(const struct dpif_backer *backer, struct ds *ds)

smap_init(&config);
if (!netdev_get_config(ofport->netdev, &config)) {
const struct smap_node **nodes;
size_t i;

nodes = smap_sort(&config);
for (i = 0; i < smap_count(&config); i++) {
const struct smap_node *node = nodes[i];
ds_put_format(ds, "%c %s=%s", i ? ',' : ':',
node->key, node->value);
const struct smap_node **nodes = smap_sort(&config);
for (size_t k = 0; k < smap_count(&config); k++) {
ds_put_format(ds, "%c %s=%s", k ? ',' : ':',
nodes[k]->key, nodes[k]->value);
}
free(nodes);
}
Expand Down
1 change: 0 additions & 1 deletion ovn/controller/ofctrl.c
Expand Up @@ -1026,7 +1026,6 @@ ofctrl_put(struct hmap *flow_table, struct shash *pending_ct_zones,
}

/* Store the barrier's xid with any newly sent ct flushes. */
struct shash_node *iter;
SHASH_FOR_EACH(iter, pending_ct_zones) {
struct ct_zone_pending_entry *ctzpe = iter->data;
if (ctzpe->state == CT_ZONE_OF_SENT && !ctzpe->of_xid) {
Expand Down
2 changes: 1 addition & 1 deletion ovn/controller/patch.c
Expand Up @@ -256,7 +256,7 @@ patch_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
* database but shouldn't. Delete them from the database. */
struct shash_node *port_node, *port_next_node;
SHASH_FOR_EACH_SAFE (port_node, port_next_node, &existing_ports) {
struct ovsrec_port *port = port_node->data;
port = port_node->data;
shash_delete(&existing_ports, port_node);
remove_port(ctx, port);
}
Expand Down
6 changes: 3 additions & 3 deletions ovn/controller/physical.c
Expand Up @@ -816,11 +816,11 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve,
remote_ofpacts_p);
}

const char *chassis;
const char *chassis_name;
const struct chassis_tunnel *prev = NULL;
SSET_FOR_EACH (chassis, &remote_chassis) {
SSET_FOR_EACH (chassis_name, &remote_chassis) {
const struct chassis_tunnel *tun
= chassis_tunnel_find(chassis);
= chassis_tunnel_find(chassis_name);
if (!tun) {
continue;
}
Expand Down

0 comments on commit 71f2127

Please sign in to comment.