On 5/20/20 12:12 AM, Ihar Hrachyshka wrote: > While the code base makes use of bool type defined in C99, we don't assume C99 > semantics for integer or pointer conversions to bool. (This is explained in > the > project coding style guide.) In C89 environments, it's important to normalize > bool variables and returned values to 1 | 0 to avoid issues down the line. > > This patch updates all functions in the tree that return bool values and that > could, in degenerate cases, return bools set to values different from 1 | 0. > > Signed-off-by: Ihar Hrachyshka <ihrac...@redhat.com> > --- > controller/pinctrl.c | 6 +++--- > ic/ovn-ic.c | 2 +- > lib/ovn-util.c | 2 +- > 3 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > index bea446c89..18b5c68dd 100644 > --- a/controller/pinctrl.c > +++ b/controller/pinctrl.c > @@ -1674,7 +1674,7 @@ pinctrl_handle_tcp_reset(struct rconn *swconn, const > struct flow *ip_flow, > static bool > is_dhcp_flags_broadcast(ovs_be16 flags) > { > - return flags & htons(DHCP_BROADCAST_FLAG); > + return !!(flags & htons(DHCP_BROADCAST_FLAG)); > } > > /* Called with in the pinctrl_handler thread context. */ > @@ -4606,7 +4606,7 @@ pinctrl_ip_mcast_handle_igmp(struct ip_mcast_snoop > *ip_ms, > break; > } > ovs_rwlock_unlock(&ip_ms->ms->rwlock); > - return group_change; > + return !!group_change; > }
Hi Ihar, Thanks for fixing this! While this is correct, I'm wondering if in the igmp/mld cases it would be cleaner to be explicit and add something like: if (mcast_snooping_add_report(ip_ms->ms, pkt_in, IP_MCAST_VLAN, port_key_data)) { group_change = true; } and respectively: if (mcast_snooping_add_mld(ip_ms->ms, pkt_in, IP_MCAST_VLAN, port_key_data)) { group_change = true; } This because the other mcast_snooping_* functions return bool, i.e., true if the IGMP/MLD packet triggered a change in the multicast group memberships. What do you think? Thanks, Dumitru > > static bool > @@ -4654,7 +4654,7 @@ pinctrl_ip_mcast_handle_mld(struct ip_mcast_snoop > *ip_ms, > break; > } > ovs_rwlock_unlock(&ip_ms->ms->rwlock); > - return group_change; > + return !!group_change; > } > > static void > diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c > index a1ed25623..9e7ceaf98 100644 > --- a/ic/ovn-ic.c > +++ b/ic/ovn-ic.c > @@ -525,7 +525,7 @@ get_router_uuid_by_sb_pb(struct ic_context *ctx, > { > const struct sbrec_port_binding *router_pb = find_peer_port(ctx, sb_pb); > if (!router_pb || !router_pb->datapath) { > - return NULL; > + return false; > } > > return smap_get_uuid(&router_pb->datapath->external_ids, > "logical-router", > diff --git a/lib/ovn-util.c b/lib/ovn-util.c > index 3482edb8d..10345b012 100644 > --- a/lib/ovn-util.c > +++ b/lib/ovn-util.c > @@ -210,7 +210,7 @@ extract_ip_addresses(const char *address, struct > lport_addresses *laddrs) > { > int ofs; > if (parse_and_store_addresses(address, laddrs, &ofs, false)) { > - return (laddrs->n_ipv4_addrs || laddrs->n_ipv6_addrs); > + return !!(laddrs->n_ipv4_addrs || laddrs->n_ipv6_addrs); > } > > return false; > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev