From mboxrd@z Thu Jan 1 00:00:00 1970 From: Serhey Popovych Subject: Re: [PATCH iproute2-next v2 5/6] utils: Introduce and use print_name_and_link() to print name@link Date: Thu, 1 Feb 2018 13:09:54 +0200 Message-ID: <605c7f63-a97e-aad0-c492-7b0ddf8b19fc@gmail.com> References: <1517335761-22095-1-git-send-email-serhe.popovych@gmail.com> <1517335761-22095-6-git-send-email-serhe.popovych@gmail.com> <2dbfa441-ab91-01b6-d6c8-0ed484cc1d79@gmail.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="JDEACsOv9S8BYOFqxhaMZNs0FEGMIVdYJ" To: David Ahern , netdev@vger.kernel.org Return-path: Received: from mail-wm0-f65.google.com ([74.125.82.65]:40142 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751766AbeBALKC (ORCPT ); Thu, 1 Feb 2018 06:10:02 -0500 Received: by mail-wm0-f65.google.com with SMTP id v123so4840904wmd.5 for ; Thu, 01 Feb 2018 03:10:02 -0800 (PST) In-Reply-To: <2dbfa441-ab91-01b6-d6c8-0ed484cc1d79@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --JDEACsOv9S8BYOFqxhaMZNs0FEGMIVdYJ Content-Type: multipart/mixed; boundary="6jhUnDPvJy2xdEzENnDvfZ98BMg9zrz9Z"; protected-headers="v1" From: Serhey Popovych To: David Ahern , netdev@vger.kernel.org Message-ID: <605c7f63-a97e-aad0-c492-7b0ddf8b19fc@gmail.com> Subject: Re: [PATCH iproute2-next v2 5/6] utils: Introduce and use print_name_and_link() to print name@link References: <1517335761-22095-1-git-send-email-serhe.popovych@gmail.com> <1517335761-22095-6-git-send-email-serhe.popovych@gmail.com> <2dbfa441-ab91-01b6-d6c8-0ed484cc1d79@gmail.com> In-Reply-To: <2dbfa441-ab91-01b6-d6c8-0ed484cc1d79@gmail.com> --6jhUnDPvJy2xdEzENnDvfZ98BMg9zrz9Z Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable David Ahern wrote: > On 1/30/18 11:09 AM, Serhey Popovych wrote: >> There is at least three places implementing same things: two in >> ipaddress.c print_linkinfo() & print_linkinfo_brief() and one in >> bridge/link.c. >> >> These two implementations diverge from each other very little: >> bridge/link.c does not support JSON output at the moment and >> print_linkinfo_brief() does not handle IFLA_LINK_NETNS case. >> >> Introduce and use print_name_and_link() routine to handle name@link >> output in all possible variations; respect IFLA_LINK_NETNS attribute t= o >> handle case when link is in different namespace; use "if%d" template >> for interface name instead of "" to share logic with other >> code (e.g. ll_name_to_index() and ll_index_to_name()) supporting such >> template. >> >> Signed-off-by: Serhey Popovych >> --- >> bridge/link.c | 13 +++---------- >> include/utils.h | 4 ++++ >> ip/ipaddress.c | 48 ++--------------------------------------------= -- >> lib/utils.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++= +++++ >> 4 files changed, 60 insertions(+), 56 deletions(-) >> >> diff --git a/bridge/link.c b/bridge/link.c >> index a11cbb1..90c9734 100644 >> --- a/bridge/link.c >> +++ b/bridge/link.c >> @@ -125,20 +125,13 @@ int print_linkinfo(const struct sockaddr_nl *who= , >> if (n->nlmsg_type =3D=3D RTM_DELLINK) >> fprintf(fp, "Deleted "); >> =20 >> - fprintf(fp, "%d: %s ", ifi->ifi_index, >> - tb[IFLA_IFNAME] ? rta_getattr_str(tb[IFLA_IFNAME]) : ""); >> + fprintf(fp, "%d: ", ifi->ifi_index); >> + >> + print_name_and_link("%s: ", COLOR_NONE, name, tb); >=20 > It only needs tb[IFLA_LINK] so just pass it. Makes the arg list > consistent with the function name too. >=20 Unfortunately not only: it uses IFLA_LINK_NETNSID too. May be adding "_rta" suffix to this routine as we did in the past when introducing get_addr_rta() and similar routines? In my opinion this is preferred than adding one more parameters to the routine. On the other hand tb[IFLA_LINK] taken by rta_getaddr_u32() (but actually it is "int" with values strictly greater than zero as implemented in kernel), so making function to catch missing tb[IFLA_LINK] would require some helper and may broke "should never happen" case when (int)rta_getattr_u32(tb[IFLA_LINK]) < 0. Currently we may call ll_index_to_name() with negative iflink. --6jhUnDPvJy2xdEzENnDvfZ98BMg9zrz9Z-- --JDEACsOv9S8BYOFqxhaMZNs0FEGMIVdYJ 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) iQEcBAEBAgAGBQJacvWGAAoJEBTawMmQ61bBxToH/jy4gU8gq8qjJUB5N5z6nkYq XNiQgOsH87cW8T2bJB7JTJUkNrwjzgEHmYFbWYoDd3Ys4jqFxN7jT7ENa4ROyi9M n+/Kzv/cU7ANvaRQkWhKVnHYMaS9XTIlWskNI26jCB+NRVxkhoSFEpW9yAD9/tPY tIrOTtPqb2uV2Yz0YtltkI35NQP+KogTuG1Iwxldpe/l4ptcQZNwEDvyOHqs4eIj xInRheDVWV+q+F14u6rPBTyU2Don/wrvRa4i0qrtDaHclxpYj3z9sgOQTobvk0uM YEmQPtXuCRXTPSbFP4qKZhE4f/ZTj0Tur+6DDkLq9V/d+zuVjgFTE93NVjTJtCU= =Y80i -----END PGP SIGNATURE----- --JDEACsOv9S8BYOFqxhaMZNs0FEGMIVdYJ--