From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Ahern Subject: Re: [PATCH iproute2-next v4 7/9] utils: Introduce and use get_ifname_rta() Date: Thu, 15 Feb 2018 12:54:35 -0700 Message-ID: References: <1518719648-30134-1-git-send-email-serhe.popovych@gmail.com> <1518719648-30134-8-git-send-email-serhe.popovych@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit To: Serhey Popovych , netdev@vger.kernel.org Return-path: Received: from mail-pg0-f66.google.com ([74.125.83.66]:36867 "EHLO mail-pg0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752053AbeBOTyj (ORCPT ); Thu, 15 Feb 2018 14:54:39 -0500 Received: by mail-pg0-f66.google.com with SMTP id o1so609873pgn.4 for ; Thu, 15 Feb 2018 11:54:38 -0800 (PST) In-Reply-To: <1518719648-30134-8-git-send-email-serhe.popovych@gmail.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 2/15/18 11:34 AM, Serhey Popovych wrote: > Be consistent in handling of IFLA_IFNAME attribute in all places: if > there is no attribute report bug to stderr and use ll_idx_n2a() as > last measure to get name in "if%u" format instead of "". > > Use check_ifname() to validate network device name: this catches both > unexpected return from kernel and ll_idx_n2a(). > > Signed-off-by: Serhey Popovych > --- > bridge/link.c | 8 ++++---- > include/utils.h | 1 + > ip/ipaddress.c | 20 ++++++++------------ > lib/utils.c | 19 +++++++++++++++++++ > 4 files changed, 32 insertions(+), 16 deletions(-) > > diff --git a/bridge/link.c b/bridge/link.c > index 870ebe0..a11cbb1 100644 > --- a/bridge/link.c > +++ b/bridge/link.c > @@ -99,9 +99,10 @@ int print_linkinfo(const struct sockaddr_nl *who, > struct nlmsghdr *n, void *arg) > { > FILE *fp = arg; > - int len = n->nlmsg_len; > struct ifinfomsg *ifi = NLMSG_DATA(n); > struct rtattr *tb[IFLA_MAX+1]; > + int len = n->nlmsg_len; > + const char *name; > > len -= NLMSG_LENGTH(sizeof(*ifi)); > if (len < 0) { > @@ -117,10 +118,9 @@ int print_linkinfo(const struct sockaddr_nl *who, > > parse_rtattr_flags(tb, IFLA_MAX, IFLA_RTA(ifi), len, NLA_F_NESTED); > > - if (tb[IFLA_IFNAME] == NULL) { > - fprintf(stderr, "BUG: nil ifname\n"); > + name = get_ifname_rta(ifi->ifi_index, tb[IFLA_IFNAME]); > + if (!name) > return -1; > - } > > if (n->nlmsg_type == RTM_DELLINK) > fprintf(fp, "Deleted "); > diff --git a/include/utils.h b/include/utils.h > index 867e540..84ca873 100644 > --- a/include/utils.h > +++ b/include/utils.h > @@ -183,6 +183,7 @@ void duparg(const char *, const char *) __attribute__((noreturn)); > void duparg2(const char *, const char *) __attribute__((noreturn)); > int check_ifname(const char *); > int get_ifname(char *, const char *); > +const char *get_ifname_rta(int ifindex, const struct rtattr *rta); > int matches(const char *arg, const char *pattern); > int inet_addr_match(const inet_prefix *a, const inet_prefix *b, int bits); > int inet_addr_match_rta(const inet_prefix *m, const struct rtattr *rta); > diff --git a/ip/ipaddress.c b/ip/ipaddress.c > index 749178d..08d2576 100644 > --- a/ip/ipaddress.c > +++ b/ip/ipaddress.c > @@ -776,12 +776,10 @@ int print_linkinfo_brief(const struct sockaddr_nl *who, > return -1; > > parse_rtattr(tb, IFLA_MAX, IFLA_RTA(ifi), len); > - if (tb[IFLA_IFNAME] == NULL) { > - fprintf(stderr, "BUG: device with ifindex %d has nil ifname\n", ifi->ifi_index); > - name = ll_idx_n2a(ifi->ifi_index); > - } else { > - name = rta_getattr_str(tb[IFLA_IFNAME]); > - } > + > + name = get_ifname_rta(ifi->ifi_index, tb[IFLA_IFNAME]); > + if (!name) > + return -1; > > if (filter.label && > (!filter.family || filter.family == AF_PACKET) && > @@ -903,12 +901,10 @@ int print_linkinfo(const struct sockaddr_nl *who, > return -1; > > parse_rtattr(tb, IFLA_MAX, IFLA_RTA(ifi), len); > - if (tb[IFLA_IFNAME] == NULL) { > - fprintf(stderr, "BUG: device with ifindex %d has nil ifname\n", ifi->ifi_index); > - name = ll_idx_n2a(ifi->ifi_index); > - } else { > - name = rta_getattr_str(tb[IFLA_IFNAME]); > - } > + > + name = get_ifname_rta(ifi->ifi_index, tb[IFLA_IFNAME]); > + if (!name) > + return -1; > > if (filter.label && > (!filter.family || filter.family == AF_PACKET) && > diff --git a/lib/utils.c b/lib/utils.c > index d86c2ee..572d42a 100644 > --- a/lib/utils.c > +++ b/lib/utils.c > @@ -871,6 +871,25 @@ int get_ifname(char *buf, const char *name) > return ret; > } > > +const char *get_ifname_rta(int ifindex, const struct rtattr *rta) > +{ > + const char *name; > + > + if (rta) { > + name = rta_getattr_str(rta); > + } else { > + fprintf(stderr, > + "BUG: device with ifindex %d has nil ifname\n", > + ifindex); > + name = ll_idx_n2a(ifindex); > + } > + > + if (check_ifname(name)) > + return NULL; > + > + return name; > +} > + > int matches(const char *cmd, const char *pattern) > { > int len = strlen(cmd); > Getting build failures with this one: $ make clean; make -j 8 ... devlink CC devlink.o CC mnlg.o LINK devlink ../lib/libutil.a(ll_map.o): In function `ll_remember_index': ll_map.c:(.text+0xf6): undefined reference to `parse_rtattr' ll_map.c:(.text+0x1c5): undefined reference to `parse_rtattr' ../lib/libutil.a(ll_map.o): In function `ll_init_map': ll_map.c:(.text+0x47c): undefined reference to `rtnl_wilddump_request' ll_map.c:(.text+0x493): undefined reference to `rtnl_dump_filter_nc'