Re: [PATCH iproute2-next v4 7/9] utils: Introduce and use get_ifname_rta()

[Serhey Popovych <serhe.popovych@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 5018 bytes --]

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.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 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".


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]