* [PATCH iproute2-next 1/6] ipaddress: Improve print_linkinfo()
2018-01-30 16:52 [PATCH iproute2-next 0/6] ipaddress: Get rid of print_linkinfo_brief() Serhey Popovych
@ 2018-01-30 16:52 ` Serhey Popovych
2018-01-30 16:52 ` [PATCH iproute2-next 2/6] ipaddress: Simplify print_linkinfo_brief() and it's usage Serhey Popovych
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Serhey Popovych @ 2018-01-30 16:52 UTC (permalink / raw)
To: netdev
There are few places to improve:
1) return -1 when entry is filtered instead of zero, which means
accept entry: ipaddress_list_flush_or_save() the only user of this
2) use ll_index_to_name() as last resort to translate name to index:
it will return name in the form "if%d" in case of failure
3) replace open coded access to IFLA_IFNAME attribute data by
RTA_DATA() with rta_getattr_str()
4) simplify ifname printing since name is never NULL, thanks to (2).
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/ipaddress.c | 30 +++++++++++++-----------------
1 file changed, 13 insertions(+), 17 deletions(-)
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 051a05f..f8fd392 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -948,14 +948,14 @@ int print_linkinfo_brief(const struct sockaddr_nl *who,
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 = "<nil>";
+ name = ll_index_to_name(ifi->ifi_index);
} else {
name = rta_getattr_str(tb[IFLA_IFNAME]);
}
if (pfilter->label &&
(!pfilter->family || pfilter->family == AF_PACKET) &&
- fnmatch(pfilter->label, RTA_DATA(tb[IFLA_IFNAME]), 0))
+ fnmatch(pfilter->label, name, 0))
return -1;
if (tb[IFLA_GROUP]) {
@@ -1057,6 +1057,7 @@ int print_linkinfo(const struct sockaddr_nl *who,
struct ifinfomsg *ifi = NLMSG_DATA(n);
struct rtattr *tb[IFLA_MAX+1];
int len = n->nlmsg_len;
+ const char *name;
unsigned int m_flag = 0;
if (n->nlmsg_type != RTM_NEWLINK && n->nlmsg_type != RTM_DELLINK)
@@ -1067,18 +1068,22 @@ int print_linkinfo(const struct sockaddr_nl *who,
return -1;
if (filter.ifindex && ifi->ifi_index != filter.ifindex)
- return 0;
+ return -1;
if (filter.up && !(ifi->ifi_flags&IFF_UP))
- return 0;
+ return -1;
parse_rtattr(tb, IFLA_MAX, IFLA_RTA(ifi), len);
- if (tb[IFLA_IFNAME] == NULL)
+ if (tb[IFLA_IFNAME] == NULL) {
fprintf(stderr, "BUG: device with ifindex %d has nil ifname\n", ifi->ifi_index);
+ name = ll_index_to_name(ifi->ifi_index);
+ } else {
+ name = rta_getattr_str(tb[IFLA_IFNAME]);
+ }
if (filter.label &&
(!filter.family || filter.family == AF_PACKET) &&
- fnmatch(filter.label, RTA_DATA(tb[IFLA_IFNAME]), 0))
- return 0;
+ fnmatch(filter.label, name, 0))
+ return -1;
if (tb[IFLA_GROUP]) {
int group = rta_getattr_u32(tb[IFLA_GROUP]);
@@ -1105,16 +1110,7 @@ int print_linkinfo(const struct sockaddr_nl *who,
print_bool(PRINT_ANY, "deleted", "Deleted ", true);
print_int(PRINT_ANY, "ifindex", "%d: ", ifi->ifi_index);
- if (tb[IFLA_IFNAME]) {
- print_color_string(PRINT_ANY,
- COLOR_IFNAME,
- "ifname", "%s",
- rta_getattr_str(tb[IFLA_IFNAME]));
- } else {
- print_null(PRINT_JSON, "ifname", NULL, NULL);
- print_color_null(PRINT_FP, COLOR_IFNAME,
- "ifname", "%s", "<nil>");
- }
+ print_color_string(PRINT_ANY, COLOR_IFNAME, "ifname", "%s", name);
if (tb[IFLA_LINK]) {
int iflink = rta_getattr_u32(tb[IFLA_LINK]);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH iproute2-next 2/6] ipaddress: Simplify print_linkinfo_brief() and it's usage
2018-01-30 16:52 [PATCH iproute2-next 0/6] ipaddress: Get rid of print_linkinfo_brief() Serhey Popovych
2018-01-30 16:52 ` [PATCH iproute2-next 1/6] ipaddress: Improve print_linkinfo() Serhey Popovych
@ 2018-01-30 16:52 ` Serhey Popovych
2018-01-30 16:52 ` [PATCH iproute2-next 3/6] lib: Correct object file dependencies Serhey Popovych
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Serhey Popovych @ 2018-01-30 16:52 UTC (permalink / raw)
To: netdev
Improve print_linkinfo_brief() and it's callers:
1) Get rid of custom @struct filter pointer @pfilter: it is NULL in
all callers anyway and global @filter is used.
2) Simplify calling code in ipaddr_list_flush_or_save() by
introducing intermediate variable of @struct nlmsghdr, drop
duplicated code: print_linkinfo_brief() never returns values other
than <= 0 so we can move print_selected_addrinfo() outside of each
block.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/ip_common.h | 3 +--
ip/ipaddress.c | 60 ++++++++++++++++++++++++--------------------------------
ip/iplink.c | 2 +-
3 files changed, 28 insertions(+), 37 deletions(-)
diff --git a/ip/ip_common.h b/ip/ip_common.h
index 3203f0c..f5adbad 100644
--- a/ip/ip_common.h
+++ b/ip/ip_common.h
@@ -30,8 +30,7 @@ int get_operstate(const char *name);
int print_linkinfo(const struct sockaddr_nl *who,
struct nlmsghdr *n, void *arg);
int print_linkinfo_brief(const struct sockaddr_nl *who,
- struct nlmsghdr *n, void *arg,
- struct link_filter *filter);
+ struct nlmsghdr *n, void *arg);
int print_addrinfo(const struct sockaddr_nl *who,
struct nlmsghdr *n, void *arg);
int print_addrlabel(const struct sockaddr_nl *who,
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index f8fd392..c15abd1 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -919,8 +919,7 @@ static void print_link_stats(FILE *fp, struct nlmsghdr *n)
}
int print_linkinfo_brief(const struct sockaddr_nl *who,
- struct nlmsghdr *n, void *arg,
- struct link_filter *pfilter)
+ struct nlmsghdr *n, void *arg)
{
FILE *fp = (FILE *)arg;
struct ifinfomsg *ifi = NLMSG_DATA(n);
@@ -937,12 +936,9 @@ int print_linkinfo_brief(const struct sockaddr_nl *who,
if (len < 0)
return -1;
- if (!pfilter)
- pfilter = &filter;
-
- if (pfilter->ifindex && ifi->ifi_index != pfilter->ifindex)
+ if (filter.ifindex && ifi->ifi_index != filter.ifindex)
return -1;
- if (pfilter->up && !(ifi->ifi_flags&IFF_UP))
+ if (filter.up && !(ifi->ifi_flags&IFF_UP))
return -1;
parse_rtattr(tb, IFLA_MAX, IFLA_RTA(ifi), len);
@@ -953,30 +949,30 @@ int print_linkinfo_brief(const struct sockaddr_nl *who,
name = rta_getattr_str(tb[IFLA_IFNAME]);
}
- if (pfilter->label &&
- (!pfilter->family || pfilter->family == AF_PACKET) &&
- fnmatch(pfilter->label, name, 0))
+ if (filter.label &&
+ (!filter.family || filter.family == AF_PACKET) &&
+ fnmatch(filter.label, name, 0))
return -1;
if (tb[IFLA_GROUP]) {
int group = rta_getattr_u32(tb[IFLA_GROUP]);
- if (pfilter->group != -1 && group != pfilter->group)
+ if (filter.group != -1 && group != filter.group)
return -1;
}
if (tb[IFLA_MASTER]) {
int master = rta_getattr_u32(tb[IFLA_MASTER]);
- if (pfilter->master > 0 && master != pfilter->master)
+ if (filter.master > 0 && master != filter.master)
return -1;
- } else if (pfilter->master > 0)
+ } else if (filter.master > 0)
return -1;
- if (pfilter->kind && match_link_kind(tb, pfilter->kind, 0))
+ if (filter.kind && match_link_kind(tb, filter.kind, 0))
return -1;
- if (pfilter->slave_kind && match_link_kind(tb, pfilter->slave_kind, 1))
+ if (filter.slave_kind && match_link_kind(tb, filter.slave_kind, 1))
return -1;
if (n->nlmsg_type == RTM_DELLINK)
@@ -1006,7 +1002,7 @@ int print_linkinfo_brief(const struct sockaddr_nl *who,
if (tb[IFLA_OPERSTATE])
print_operstate(fp, rta_getattr_u8(tb[IFLA_OPERSTATE]));
- if (pfilter->family == AF_PACKET) {
+ if (filter.family == AF_PACKET) {
SPRINT_BUF(b1);
if (tb[IFLA_ADDRESS]) {
@@ -1020,7 +1016,7 @@ int print_linkinfo_brief(const struct sockaddr_nl *who,
}
}
- if (pfilter->family == AF_PACKET) {
+ if (filter.family == AF_PACKET) {
print_link_flags(fp, ifi->ifi_flags, m_flag);
print_string(PRINT_FP, NULL, "%s", "\n");
}
@@ -2188,25 +2184,21 @@ static int ipaddr_list_flush_or_save(int argc, char **argv, int action)
ipaddr_filter(&linfo, ainfo);
for (l = linfo.head; l; l = l->next) {
- int res = 0;
- struct ifinfomsg *ifi = NLMSG_DATA(&l->h);
+ struct nlmsghdr *n = &l->h;
+ struct ifinfomsg *ifi = NLMSG_DATA(n);
+ int res;
open_json_object(NULL);
- if (brief) {
- if (print_linkinfo_brief(NULL, &l->h,
- stdout, NULL) == 0)
- if (filter.family != AF_PACKET)
- print_selected_addrinfo(ifi,
- ainfo->head,
- stdout);
- } else if (no_link ||
- (res = print_linkinfo(NULL, &l->h, stdout)) >= 0) {
- if (filter.family != AF_PACKET)
- print_selected_addrinfo(ifi,
- ainfo->head, stdout);
- if (res > 0 && !do_link && show_stats)
- print_link_stats(stdout, &l->h);
- }
+ if (brief)
+ res = print_linkinfo_brief(NULL, n, stdout);
+ else if (no_link)
+ res = 0;
+ else
+ res = print_linkinfo(NULL, n, stdout);
+ if (res >= 0 && filter.family != AF_PACKET)
+ print_selected_addrinfo(ifi, ainfo->head, stdout);
+ if (res > 0 && !do_link && show_stats)
+ print_link_stats(stdout, n);
close_json_object();
}
fflush(stdout);
diff --git a/ip/iplink.c b/ip/iplink.c
index 230f4c5..882cd13 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -1077,7 +1077,7 @@ int iplink_get(unsigned int flags, char *name, __u32 filt_mask)
open_json_object(NULL);
if (brief)
- print_linkinfo_brief(NULL, answer, stdout, NULL);
+ print_linkinfo_brief(NULL, answer, stdout);
else
print_linkinfo(NULL, answer, stdout);
close_json_object();
--
1.7.10.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH iproute2-next 3/6] lib: Correct object file dependencies
2018-01-30 16:52 [PATCH iproute2-next 0/6] ipaddress: Get rid of print_linkinfo_brief() Serhey Popovych
2018-01-30 16:52 ` [PATCH iproute2-next 1/6] ipaddress: Improve print_linkinfo() Serhey Popovych
2018-01-30 16:52 ` [PATCH iproute2-next 2/6] ipaddress: Simplify print_linkinfo_brief() and it's usage Serhey Popovych
@ 2018-01-30 16:52 ` Serhey Popovych
2018-01-30 16:52 ` [PATCH iproute2-next 4/6] utils: Introduce and use get_ifname_rta() Serhey Popovych
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Serhey Popovych @ 2018-01-30 16:52 UTC (permalink / raw)
To: netdev
Neither internal libnetlink nor libgenl depends on ll_map.o: prepare for
upcoming changes that brings much more cleaner dependency between
utils.o and ll_map.o.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
lib/Makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/Makefile b/lib/Makefile
index 7b34ed5..bab8cbf 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -3,11 +3,11 @@ include ../config.mk
CFLAGS += -fPIC
-UTILOBJ = utils.o rt_names.o ll_types.o ll_proto.o ll_addr.o \
+UTILOBJ = utils.o rt_names.o ll_map.o ll_types.o ll_proto.o ll_addr.o \
inet_proto.o namespace.o json_writer.o json_print.o \
names.o color.o bpf.o exec.o fs.o
-NLOBJ=libgenl.o ll_map.o libnetlink.o
+NLOBJ=libgenl.o libnetlink.o
all: libnetlink.a libutil.a
--
1.7.10.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH iproute2-next 4/6] utils: Introduce and use get_ifname_rta()
2018-01-30 16:52 [PATCH iproute2-next 0/6] ipaddress: Get rid of print_linkinfo_brief() Serhey Popovych
` (2 preceding siblings ...)
2018-01-30 16:52 ` [PATCH iproute2-next 3/6] lib: Correct object file dependencies Serhey Popovych
@ 2018-01-30 16:52 ` Serhey Popovych
2018-01-30 16:52 ` [PATCH iproute2-next 5/6] utils: Introduce and use print_name_and_link() to print name@link Serhey Popovych
2018-01-30 16:52 ` [PATCH iproute2-next 6/6] ipaddress: Get rid of print_linkinfo_brief() Serhey Popovych
5 siblings, 0 replies; 9+ messages in thread
From: Serhey Popovych @ 2018-01-30 16:52 UTC (permalink / raw)
To: netdev
Be consistent in handling of IFLA_IFNAME attribute in all places: if
there is no attribute report bug to stderr and use ll_index_to_name() as
last measure to get name in "if%d" format instead of "<nil>".
Use check_ifname() to validate network device name: this catches both
unexpected return from kernel and ll_index_to_name().
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
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 0394268..5738c97 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -173,6 +173,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 c15abd1..1797927 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -942,12 +942,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_index_to_name(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) &&
@@ -1069,12 +1067,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_index_to_name(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 8e15625..29e2d84 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_index_to_name(ifindex);
+ }
+
+ if (check_ifname(name))
+ return NULL;
+
+ return name;
+}
+
int matches(const char *cmd, const char *pattern)
{
int len = strlen(cmd);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH iproute2-next 5/6] utils: Introduce and use print_name_and_link() to print name@link
2018-01-30 16:52 [PATCH iproute2-next 0/6] ipaddress: Get rid of print_linkinfo_brief() Serhey Popovych
` (3 preceding siblings ...)
2018-01-30 16:52 ` [PATCH iproute2-next 4/6] utils: Introduce and use get_ifname_rta() Serhey Popovych
@ 2018-01-30 16:52 ` Serhey Popovych
2018-01-30 16:52 ` [PATCH iproute2-next 6/6] ipaddress: Get rid of print_linkinfo_brief() Serhey Popovych
5 siblings, 0 replies; 9+ messages in thread
From: Serhey Popovych @ 2018-01-30 16:52 UTC (permalink / raw)
To: netdev
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 to
handle case when link is in different namespace; use "if%d" template
for interface name instead of "<nil>" 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 <serhe.popovych@gmail.com>
---
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 == RTM_DELLINK)
fprintf(fp, "Deleted ");
- fprintf(fp, "%d: %s ", ifi->ifi_index,
- tb[IFLA_IFNAME] ? rta_getattr_str(tb[IFLA_IFNAME]) : "<nil>");
+ fprintf(fp, "%d: ", ifi->ifi_index);
+
+ print_name_and_link("%s: ", COLOR_NONE, name, tb);
if (tb[IFLA_OPERSTATE])
print_operstate(fp, rta_getattr_u8(tb[IFLA_OPERSTATE]));
- if (tb[IFLA_LINK]) {
- int iflink = rta_getattr_u32(tb[IFLA_LINK]);
-
- fprintf(fp, "@%s: ",
- iflink ? ll_index_to_name(iflink) : "NONE");
- } else
- fprintf(fp, ": ");
-
print_link_flags(fp, ifi->ifi_flags);
if (tb[IFLA_MTU])
diff --git a/include/utils.h b/include/utils.h
index 5738c97..d217073 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -12,6 +12,7 @@
#include "libnetlink.h"
#include "ll_map.h"
#include "rtm_map.h"
+#include "json_print.h"
extern int preferred_family;
extern int human_readable;
@@ -240,6 +241,9 @@ void print_escape_buf(const __u8 *buf, size_t len, const char *escape);
int print_timestamp(FILE *fp);
void print_nlmsg_timestamp(FILE *fp, const struct nlmsghdr *n);
+unsigned int print_name_and_link(const char *fmt, enum color_attr color,
+ const char *name, struct rtattr *tb[]);
+
#define BIT(nr) (1UL << (nr))
#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 1797927..5afc9d4 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -926,7 +926,6 @@ int print_linkinfo_brief(const struct sockaddr_nl *who,
struct rtattr *tb[IFLA_MAX+1];
int len = n->nlmsg_len;
const char *name;
- char buf[32] = { 0, };
unsigned int m_flag = 0;
if (n->nlmsg_type != RTM_NEWLINK && n->nlmsg_type != RTM_DELLINK)
@@ -976,26 +975,7 @@ int print_linkinfo_brief(const struct sockaddr_nl *who,
if (n->nlmsg_type == RTM_DELLINK)
print_bool(PRINT_ANY, "deleted", "Deleted ", true);
- if (tb[IFLA_LINK]) {
- SPRINT_BUF(b1);
- int iflink = rta_getattr_u32(tb[IFLA_LINK]);
-
- if (iflink == 0) {
- snprintf(buf, sizeof(buf), "%s@NONE", name);
- print_null(PRINT_JSON, "link", NULL, NULL);
- } else {
- const char *link = ll_idx_n2a(iflink, b1);
-
- print_string(PRINT_JSON, "link", NULL, link);
- snprintf(buf, sizeof(buf), "%s@%s", name, link);
- m_flag = ll_index_to_flags(iflink);
- m_flag = !(m_flag & IFF_UP);
- }
- } else
- snprintf(buf, sizeof(buf), "%s", name);
-
- print_string(PRINT_FP, NULL, "%-16s ", buf);
- print_string(PRINT_JSON, "ifname", NULL, name);
+ m_flag = print_name_and_link("%-16s ", COLOR_NONE, name, tb);
if (tb[IFLA_OPERSTATE])
print_operstate(fp, rta_getattr_u8(tb[IFLA_OPERSTATE]));
@@ -1102,31 +1082,7 @@ int print_linkinfo(const struct sockaddr_nl *who,
print_bool(PRINT_ANY, "deleted", "Deleted ", true);
print_int(PRINT_ANY, "ifindex", "%d: ", ifi->ifi_index);
- print_color_string(PRINT_ANY, COLOR_IFNAME, "ifname", "%s", name);
-
- if (tb[IFLA_LINK]) {
- int iflink = rta_getattr_u32(tb[IFLA_LINK]);
-
- if (iflink == 0)
- print_null(PRINT_ANY, "link", "@%s: ", "NONE");
- else {
- if (tb[IFLA_LINK_NETNSID])
- print_int(PRINT_ANY,
- "link_index", "@if%d: ", iflink);
- else {
- SPRINT_BUF(b1);
-
- print_string(PRINT_ANY,
- "link",
- "@%s: ",
- ll_idx_n2a(iflink, b1));
- m_flag = ll_index_to_flags(iflink);
- m_flag = !(m_flag & IFF_UP);
- }
- }
- } else {
- print_string(PRINT_FP, NULL, ": ", NULL);
- }
+ m_flag = print_name_and_link("%s: ", COLOR_IFNAME, name, tb);
print_link_flags(fp, ifi->ifi_flags, m_flag);
if (tb[IFLA_MTU])
diff --git a/lib/utils.c b/lib/utils.c
index 29e2d84..c39ee50 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -1260,6 +1260,57 @@ int print_timestamp(FILE *fp)
return 0;
}
+unsigned int print_name_and_link(const char *fmt, enum color_attr color,
+ const char *name, struct rtattr *tb[])
+{
+ const char *link = NULL;
+ unsigned int m_flag = 0;
+ SPRINT_BUF(b1);
+ SPRINT_BUF(b2);
+
+ if (tb[IFLA_LINK]) {
+ int iflink = rta_getattr_u32(tb[IFLA_LINK]);
+
+ if (iflink) {
+ if (tb[IFLA_LINK_NETNSID]) {
+ if (is_json_context()) {
+ print_int(PRINT_JSON,
+ "link_index", NULL, iflink);
+ } else {
+ snprintf(b2, sizeof(b2),
+ "if%d", iflink);
+ link = b2;
+ }
+ } else {
+ link = ll_index_to_name(iflink);
+
+ if (is_json_context()) {
+ print_string(PRINT_JSON,
+ "link", NULL, link);
+ link = NULL;
+ }
+
+ m_flag = ll_index_to_flags(iflink);
+ m_flag = !(m_flag & IFF_UP);
+ }
+ } else {
+ if (is_json_context())
+ print_null(PRINT_JSON, "link", NULL, NULL);
+ else
+ link = "NONE";
+ }
+
+ if (link) {
+ snprintf(b1, sizeof(b1), "%s@%s", name, link);
+ name = b1;
+ }
+ }
+
+ print_color_string(PRINT_ANY, color, "ifname", fmt, name);
+
+ return m_flag;
+}
+
int cmdlineno;
/* Like glibc getline but handle continuation lines and comments */
--
1.7.10.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH iproute2-next 6/6] ipaddress: Get rid of print_linkinfo_brief()
2018-01-30 16:52 [PATCH iproute2-next 0/6] ipaddress: Get rid of print_linkinfo_brief() Serhey Popovych
` (4 preceding siblings ...)
2018-01-30 16:52 ` [PATCH iproute2-next 5/6] utils: Introduce and use print_name_and_link() to print name@link Serhey Popovych
@ 2018-01-30 16:52 ` Serhey Popovych
2018-01-30 17:24 ` Stephen Hemminger
5 siblings, 1 reply; 9+ messages in thread
From: Serhey Popovych @ 2018-01-30 16:52 UTC (permalink / raw)
To: netdev
It's functionality can be fully accomplished by print_linkinfo(): no
need to duplicate code.
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/ip_common.h | 2 -
ip/ipaddress.c | 126 ++++++++++++++------------------------------------------
ip/iplink.c | 5 +--
3 files changed, 33 insertions(+), 100 deletions(-)
diff --git a/ip/ip_common.h b/ip/ip_common.h
index f5adbad..a7bbf1d 100644
--- a/ip/ip_common.h
+++ b/ip/ip_common.h
@@ -29,8 +29,6 @@ struct link_filter {
int get_operstate(const char *name);
int print_linkinfo(const struct sockaddr_nl *who,
struct nlmsghdr *n, void *arg);
-int print_linkinfo_brief(const struct sockaddr_nl *who,
- struct nlmsghdr *n, void *arg);
int print_addrinfo(const struct sockaddr_nl *who,
struct nlmsghdr *n, void *arg);
int print_addrlabel(const struct sockaddr_nl *who,
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 5afc9d4..88e61e4 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -918,90 +918,6 @@ static void print_link_stats(FILE *fp, struct nlmsghdr *n)
fprintf(fp, "%s", _SL_);
}
-int print_linkinfo_brief(const struct sockaddr_nl *who,
- struct nlmsghdr *n, void *arg)
-{
- FILE *fp = (FILE *)arg;
- struct ifinfomsg *ifi = NLMSG_DATA(n);
- struct rtattr *tb[IFLA_MAX+1];
- int len = n->nlmsg_len;
- const char *name;
- unsigned int m_flag = 0;
-
- if (n->nlmsg_type != RTM_NEWLINK && n->nlmsg_type != RTM_DELLINK)
- return -1;
-
- len -= NLMSG_LENGTH(sizeof(*ifi));
- if (len < 0)
- return -1;
-
- if (filter.ifindex && ifi->ifi_index != filter.ifindex)
- return -1;
- if (filter.up && !(ifi->ifi_flags&IFF_UP))
- return -1;
-
- parse_rtattr(tb, IFLA_MAX, IFLA_RTA(ifi), len);
-
- name = get_ifname_rta(ifi->ifi_index, tb[IFLA_IFNAME]);
- if (!name)
- return -1;
-
- if (filter.label &&
- (!filter.family || filter.family == AF_PACKET) &&
- fnmatch(filter.label, name, 0))
- return -1;
-
- if (tb[IFLA_GROUP]) {
- int group = rta_getattr_u32(tb[IFLA_GROUP]);
-
- if (filter.group != -1 && group != filter.group)
- return -1;
- }
-
- if (tb[IFLA_MASTER]) {
- int master = rta_getattr_u32(tb[IFLA_MASTER]);
-
- if (filter.master > 0 && master != filter.master)
- return -1;
- } else if (filter.master > 0)
- return -1;
-
- if (filter.kind && match_link_kind(tb, filter.kind, 0))
- return -1;
-
- if (filter.slave_kind && match_link_kind(tb, filter.slave_kind, 1))
- return -1;
-
- if (n->nlmsg_type == RTM_DELLINK)
- print_bool(PRINT_ANY, "deleted", "Deleted ", true);
-
- m_flag = print_name_and_link("%-16s ", COLOR_NONE, name, tb);
-
- if (tb[IFLA_OPERSTATE])
- print_operstate(fp, rta_getattr_u8(tb[IFLA_OPERSTATE]));
-
- if (filter.family == AF_PACKET) {
- SPRINT_BUF(b1);
-
- if (tb[IFLA_ADDRESS]) {
- print_color_string(PRINT_ANY, COLOR_MAC,
- "address", "%s ",
- ll_addr_n2a(
- RTA_DATA(tb[IFLA_ADDRESS]),
- RTA_PAYLOAD(tb[IFLA_ADDRESS]),
- ifi->ifi_type,
- b1, sizeof(b1)));
- }
- }
-
- if (filter.family == AF_PACKET) {
- print_link_flags(fp, ifi->ifi_flags, m_flag);
- print_string(PRINT_FP, NULL, "%s", "\n");
- }
- fflush(fp);
- return 0;
-}
-
static const char *link_events[] = {
[IFLA_EVENT_NONE] = "NONE",
[IFLA_EVENT_REBOOT] = "REBOOT",
@@ -1033,6 +949,7 @@ int print_linkinfo(const struct sockaddr_nl *who,
int len = n->nlmsg_len;
const char *name;
unsigned int m_flag = 0;
+ SPRINT_BUF(b1);
if (n->nlmsg_type != RTM_NEWLINK && n->nlmsg_type != RTM_DELLINK)
return 0;
@@ -1081,6 +998,34 @@ int print_linkinfo(const struct sockaddr_nl *who,
if (n->nlmsg_type == RTM_DELLINK)
print_bool(PRINT_ANY, "deleted", "Deleted ", true);
+ if (brief) {
+ print_name_and_link("%-16s ", COLOR_NONE, name, tb);
+
+ if (tb[IFLA_OPERSTATE])
+ print_operstate(fp, rta_getattr_u8(tb[IFLA_OPERSTATE]));
+
+ if (filter.family == AF_PACKET) {
+ if (tb[IFLA_ADDRESS]) {
+ struct rtattr *rta = tb[IFLA_ADDRESS];
+
+ print_color_string(PRINT_ANY,
+ COLOR_MAC,
+ "address",
+ "%s ",
+ ll_addr_n2a(RTA_DATA(rta),
+ RTA_PAYLOAD(rta),
+ ifi->ifi_type,
+ b1, sizeof(b1)));
+ }
+
+ print_link_flags(fp, ifi->ifi_flags, m_flag);
+ print_string(PRINT_FP, NULL, "%s", "\n");
+ }
+
+ fflush(fp);
+ return 0;
+ }
+
print_int(PRINT_ANY, "ifindex", "%d: ", ifi->ifi_index);
m_flag = print_name_and_link("%s: ", COLOR_IFNAME, name, tb);
print_link_flags(fp, ifi->ifi_flags, m_flag);
@@ -1097,8 +1042,6 @@ int print_linkinfo(const struct sockaddr_nl *who,
"qdisc %s ",
rta_getattr_str(tb[IFLA_QDISC]));
if (tb[IFLA_MASTER]) {
- SPRINT_BUF(b1);
-
print_string(PRINT_ANY,
"master",
"master %s ",
@@ -1112,7 +1055,6 @@ int print_linkinfo(const struct sockaddr_nl *who,
print_linkmode(fp, tb[IFLA_LINKMODE]);
if (tb[IFLA_GROUP]) {
- SPRINT_BUF(b1);
int group = rta_getattr_u32(tb[IFLA_GROUP]);
print_string(PRINT_ANY,
@@ -1279,7 +1221,7 @@ int print_linkinfo(const struct sockaddr_nl *who,
close_json_array(PRINT_JSON, NULL);
}
- print_string(PRINT_FP, NULL, "\n", NULL);
+ print_string(PRINT_FP, NULL, "%s", "\n");
fflush(fp);
return 1;
}
@@ -2138,14 +2080,10 @@ static int ipaddr_list_flush_or_save(int argc, char **argv, int action)
for (l = linfo.head; l; l = l->next) {
struct nlmsghdr *n = &l->h;
struct ifinfomsg *ifi = NLMSG_DATA(n);
- int res;
+ int res = 0;
open_json_object(NULL);
- if (brief)
- res = print_linkinfo_brief(NULL, n, stdout);
- else if (no_link)
- res = 0;
- else
+ if (brief || !no_link)
res = print_linkinfo(NULL, n, stdout);
if (res >= 0 && filter.family != AF_PACKET)
print_selected_addrinfo(ifi, ainfo->head, stdout);
diff --git a/ip/iplink.c b/ip/iplink.c
index 882cd13..6f8f7cd 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -1076,10 +1076,7 @@ int iplink_get(unsigned int flags, char *name, __u32 filt_mask)
return -2;
open_json_object(NULL);
- if (brief)
- print_linkinfo_brief(NULL, answer, stdout);
- else
- print_linkinfo(NULL, answer, stdout);
+ print_linkinfo(NULL, answer, stdout);
close_json_object();
free(answer);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH iproute2-next 6/6] ipaddress: Get rid of print_linkinfo_brief()
2018-01-30 16:52 ` [PATCH iproute2-next 6/6] ipaddress: Get rid of print_linkinfo_brief() Serhey Popovych
@ 2018-01-30 17:24 ` Stephen Hemminger
2018-01-30 18:10 ` Serhey Popovych
0 siblings, 1 reply; 9+ messages in thread
From: Stephen Hemminger @ 2018-01-30 17:24 UTC (permalink / raw)
To: Serhey Popovych; +Cc: netdev
On Tue, 30 Jan 2018 18:52:48 +0200
Serhey Popovych <serhe.popovych@gmail.com> wrote:
> + if (brief) {
> + print_name_and_link("%-16s ", COLOR_NONE, name, tb);
> +
> + if (tb[IFLA_OPERSTATE])
> + print_operstate(fp, rta_getattr_u8(tb[IFLA_OPERSTATE]));
> +
> + if (filter.family == AF_PACKET) {
> + if (tb[IFLA_ADDRESS]) {
> + struct rtattr *rta = tb[IFLA_ADDRESS];
> +
> + print_color_string(PRINT_ANY,
> + COLOR_MAC,
> + "address",
> + "%s ",
> + ll_addr_n2a(RTA_DATA(rta),
> + RTA_PAYLOAD(rta),
> + ifi->ifi_type,
> + b1, sizeof(b1)));
> + }
> +
> + print_link_flags(fp, ifi->ifi_flags, m_flag);
> + print_string(PRINT_FP, NULL, "%s", "\n");
> + }
> +
> + fflush(fp);
> + return 0;
> + }
To keep function shorter and therefore more readable, why not:
if (brief)
return print_linkinfo_brief(fp, ifi, tb);
And put this if branch in new version of print_linkinfo_brief.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH iproute2-next 6/6] ipaddress: Get rid of print_linkinfo_brief()
2018-01-30 17:24 ` Stephen Hemminger
@ 2018-01-30 18:10 ` Serhey Popovych
0 siblings, 0 replies; 9+ messages in thread
From: Serhey Popovych @ 2018-01-30 18:10 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
[-- Attachment #1.1: Type: text/plain, Size: 1168 bytes --]
Stephen Hemminger wrote:
> On Tue, 30 Jan 2018 18:52:48 +0200
> Serhey Popovych <serhe.popovych@gmail.com> wrote:
>
>> + if (brief) {
>> + print_name_and_link("%-16s ", COLOR_NONE, name, tb);
>> +
>> + if (tb[IFLA_OPERSTATE])
>> + print_operstate(fp, rta_getattr_u8(tb[IFLA_OPERSTATE]));
>> +
>> + if (filter.family == AF_PACKET) {
>> + if (tb[IFLA_ADDRESS]) {
>> + struct rtattr *rta = tb[IFLA_ADDRESS];
>> +
>> + print_color_string(PRINT_ANY,
>> + COLOR_MAC,
>> + "address",
>> + "%s ",
>> + ll_addr_n2a(RTA_DATA(rta),
>> + RTA_PAYLOAD(rta),
>> + ifi->ifi_type,
>> + b1, sizeof(b1)));
>> + }
>> +
>> + print_link_flags(fp, ifi->ifi_flags, m_flag);
>> + print_string(PRINT_FP, NULL, "%s", "\n");
>> + }
>> +
>> + fflush(fp);
>> + return 0;
>> + }
>
> To keep function shorter and therefore more readable, why not:
>
> if (brief)
> return print_linkinfo_brief(fp, ifi, tb);
>
> And put this if branch in new version of print_linkinfo_brief.
>
Agree, will make it static and branch as suggested. Thanks.
Addressed in v2.
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread