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 "<nil>".
>>
>> Use check_ifname() to validate network device name: this catches both
>> unexpected return from kernel and ll_idx_n2a().
>>
>> Signed-off-by: Serhey Popovych <serhe.popov...@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 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'
>
> Corrected, sorry for that. It is actually caused by "lib: Correct object file dependencies".
signature.asc
Description: OpenPGP digital signature
