From mboxrd@z Thu Jan 1 00:00:00 1970 From: Serhey Popovych Subject: Re: [PATCH iproute2-next v4 7/9] utils: Introduce and use get_ifname_rta() Date: Thu, 15 Feb 2018 23:25:17 +0200 Message-ID: <89f79e22-75db-f680-b5c6-58272dd452ff@gmail.com> 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: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="uqyajfpHnNX8qXx1Pz3JfEdAA0Sj2vHnI" To: David Ahern , netdev@vger.kernel.org Return-path: Received: from mail-lf0-f68.google.com ([209.85.215.68]:37301 "EHLO mail-lf0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755950AbeBOVZ0 (ORCPT ); Thu, 15 Feb 2018 16:25:26 -0500 Received: by mail-lf0-f68.google.com with SMTP id f137so1486415lfe.4 for ; Thu, 15 Feb 2018 13:25:25 -0800 (PST) In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --uqyajfpHnNX8qXx1Pz3JfEdAA0Sj2vHnI Content-Type: multipart/mixed; boundary="oLbwsmAL0kjXYq7xtLr2uZJC6JjggeZ3A"; protected-headers="v1" From: Serhey Popovych To: David Ahern , netdev@vger.kernel.org Message-ID: <89f79e22-75db-f680-b5c6-58272dd452ff@gmail.com> Subject: Re: [PATCH iproute2-next v4 7/9] utils: Introduce and use get_ifname_rta() References: <1518719648-30134-1-git-send-email-serhe.popovych@gmail.com> <1518719648-30134-8-git-send-email-serhe.popovych@gmail.com> In-Reply-To: --oLbwsmAL0kjXYq7xtLr2uZJC6JjggeZ3A Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable David Ahern wrote: > 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 =3D arg; >> - int len =3D n->nlmsg_len; >> struct ifinfomsg *ifi =3D NLMSG_DATA(n); >> struct rtattr *tb[IFLA_MAX+1]; >> + int len =3D n->nlmsg_len; >> + const char *name; >> =20 >> len -=3D NLMSG_LENGTH(sizeof(*ifi)); >> if (len < 0) { >> @@ -117,10 +118,9 @@ int print_linkinfo(const struct sockaddr_nl *who,= >> =20 >> parse_rtattr_flags(tb, IFLA_MAX, IFLA_RTA(ifi), len, NLA_F_NESTED); >> =20 >> - if (tb[IFLA_IFNAME] =3D=3D NULL) { >> - fprintf(stderr, "BUG: nil ifname\n"); >> + name =3D get_ifname_rta(ifi->ifi_index, tb[IFLA_IFNAME]); >> + if (!name) >> return -1; >> - } >> =20 >> if (n->nlmsg_type =3D=3D 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 *) __attribut= e__((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 b= its); >> int inet_addr_match_rta(const inet_prefix *m, const struct rtattr *rt= a); >> 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_n= l *who, >> return -1; >> =20 >> parse_rtattr(tb, IFLA_MAX, IFLA_RTA(ifi), len); >> - if (tb[IFLA_IFNAME] =3D=3D NULL) { >> - fprintf(stderr, "BUG: device with ifindex %d has nil ifname\n", ifi= ->ifi_index); >> - name =3D ll_idx_n2a(ifi->ifi_index); >> - } else { >> - name =3D rta_getattr_str(tb[IFLA_IFNAME]); >> - } >> + >> + name =3D get_ifname_rta(ifi->ifi_index, tb[IFLA_IFNAME]); >> + if (!name) >> + return -1; >> =20 >> if (filter.label && >> (!filter.family || filter.family =3D=3D AF_PACKET) && >> @@ -903,12 +901,10 @@ int print_linkinfo(const struct sockaddr_nl *who= , >> return -1; >> =20 >> parse_rtattr(tb, IFLA_MAX, IFLA_RTA(ifi), len); >> - if (tb[IFLA_IFNAME] =3D=3D NULL) { >> - fprintf(stderr, "BUG: device with ifindex %d has nil ifname\n", ifi= ->ifi_index); >> - name =3D ll_idx_n2a(ifi->ifi_index); >> - } else { >> - name =3D rta_getattr_str(tb[IFLA_IFNAME]); >> - } >> + >> + name =3D get_ifname_rta(ifi->ifi_index, tb[IFLA_IFNAME]); >> + if (!name) >> + return -1; >> =20 >> if (filter.label && >> (!filter.family || filter.family =3D=3D 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; >> } >> =20 >> +const char *get_ifname_rta(int ifindex, const struct rtattr *rta) >> +{ >> + const char *name; >> + >> + if (rta) { >> + name =3D rta_getattr_str(rta); >> + } else { >> + fprintf(stderr, >> + "BUG: device with ifindex %d has nil ifname\n", >> + ifindex); >> + name =3D ll_idx_n2a(ifindex); >> + } >> + >> + if (check_ifname(name)) >> + return NULL; >> + >> + return name; >> +} >> + >> int matches(const char *cmd, const char *pattern) >> { >> int len =3D strlen(cmd); >> >=20 >=20 > Getting build failures with this one: > $ make clean; make -j 8 > ... >=20 > 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' >=20 >=20 Corrected, sorry for that. It is actually caused by "lib: Correct object file dependencies". --oLbwsmAL0kjXYq7xtLr2uZJC6JjggeZ3A-- --uqyajfpHnNX8qXx1Pz3JfEdAA0Sj2vHnI Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQEcBAEBAgAGBQJahfrCAAoJEBTawMmQ61bBFQIIAMVodvt+RkYq84nJv+SApT1L KaRikTWoS1l8uTL8EhgwSrijLqEoJfLpnyvdocCqJGxKnxC6evu4V7g2gA+zYG8T xhwii2o/V/5v6Jn0gXa38TBfPNl9co1SzA4nwIcrd7MRuOKzT4Kr79eGbuPKo4zO 9XzFQJNxu4OW2UYG1tFpF4LTjpyblPQ+SafUieHmsJwVY6lt2EY0WBnUX9b7ZtHj 800GRz5QabV8YoAWnV18a1a7HnqoEyWb8XO14u0A147TeQCICrQ1fM7bDi4gW0V8 8sTUquwSscsbwWBUwTeBGHmubHhej6pMZ7ae5ujPul3kiw9iIyiu95iF6/ay+Wg= =/+w3 -----END PGP SIGNATURE----- --uqyajfpHnNX8qXx1Pz3JfEdAA0Sj2vHnI--