[PATCH] fix if_indextoname error case

[PATCH] fix if_indextoname error case

[Daniel Sabogal]

posix requires errno to be set to ENXIO if the interface does not exist.
linux returns ENODEV instead so we handle this.
---
 src/network/if_indextoname.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/network/if_indextoname.c b/src/network/if_indextoname.c
index 6ee7f13..3b368bf 100644
--- a/src/network/if_indextoname.c
+++ b/src/network/if_indextoname.c
@@ -3,6 +3,7 @@
 #include <sys/socket.h>
 #include <sys/ioctl.h>
 #include <string.h>
+#include <errno.h>
 #include "syscall.h"
 
 char *if_indextoname(unsigned index, char *name)
@@ -14,5 +15,9 @@ char *if_indextoname(unsigned index, char *name)
 	ifr.ifr_ifindex = index;
 	r = ioctl(fd, SIOCGIFNAME, &ifr);
 	__syscall(SYS_close, fd);
-	return r < 0 ? 0 : strncpy(name, ifr.ifr_name, IF_NAMESIZE);
+	if (r < 0) {
+		if (errno == ENODEV) errno = ENXIO;
+		return 0;
+	}
+	return strncpy(name, ifr.ifr_name, IF_NAMESIZE);
 }
-- 
2.10.0

[PATCH] fix type for ifc_ifcu.ifcu_buf field of struct ifconf

[Daniel Sabogal]

linux, glibc, freebsd, and openbsd use a character pointer for this field
---
 include/net/if.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/if.h b/include/net/if.h
index 1a4059d..76a2528 100644
--- a/include/net/if.h
+++ b/include/net/if.h
@@ -116,7 +116,7 @@ struct ifreq {
 struct ifconf {
 	int ifc_len;		
 	union {
-		void *ifcu_buf;
+		char *ifcu_buf;
 		struct ifreq *ifcu_req;
 	} ifc_ifcu;
 };
-- 
2.10.0

Re: [PATCH] fix type for ifc_ifcu.ifcu_buf field of struct ifconf

[Alexander Monakov]

On Thu, 15 Sep 2016, Daniel Sabogal wrote:
> linux, glibc, freebsd, and openbsd use a character pointer for this field
> --- a/include/net/if.h
> +++ b/include/net/if.h
> @@ -116,7 +116,7 @@ struct ifreq {
>  struct ifconf {
>  	int ifc_len;		
>  	union {
> -		void *ifcu_buf;
> +		char *ifcu_buf;

Declaration of 'ifru_data' in that header has the same issue.

Thanks.
Alexander

Re: [PATCH] fix type for ifc_ifcu.ifcu_buf field of struct ifconf

[Daniel Sabogal]

On Thu, Sep 15, 2016 at 4:25 PM, Alexander Monakov <amonakov@ispras.ru> wrote:
> On Thu, 15 Sep 2016, Daniel Sabogal wrote:
>> linux, glibc, freebsd, and openbsd use a character pointer for this field
>> --- a/include/net/if.h
>> +++ b/include/net/if.h
>> @@ -116,7 +116,7 @@ struct ifreq {
>>  struct ifconf {
>>       int ifc_len;
>>       union {
>> -             void *ifcu_buf;
>> +             char *ifcu_buf;
>
> Declaration of 'ifru_data' in that header has the same issue.

I originally considered changing this one but had left it out since
linux disagree on the type (void *).

Re: [PATCH] fix type for ifc_ifcu.ifcu_buf field of struct ifconf

[Alexander Monakov]

On Thu, 15 Sep 2016, Daniel Sabogal wrote:
> On Thu, Sep 15, 2016 at 4:25 PM, Alexander Monakov <amonakov@ispras.ru> wrote:
> > Declaration of 'ifru_data' in that header has the same issue.
> 
> I originally considered changing this one but had left it out since
> linux disagree on the type (void *).

Ah. From looking at pre-git Linux history, it appears that it had
'char *', but then it was changed to 'void *' in June 2004 by commit
'[PATCH] sparse: ->ifr_data annotation' by Al Viro, causing a divergence
with the Glibc interface.

FreeBSD also has 'char *' (caddr_t) there, and netdevice(7) manpage documents
that the Linux interface has 'char *' as well. Thus, the kernel uapi header is
the odd one out.

Please wait for further feedback from Rich.

Thanks.
Alexander

Re: [PATCH] fix type for ifc_ifcu.ifcu_buf field of struct ifconf

[Rich Felker]

On Fri, Sep 16, 2016 at 11:12:14AM +0300, Alexander Monakov wrote:
> On Thu, 15 Sep 2016, Daniel Sabogal wrote:
> > On Thu, Sep 15, 2016 at 4:25 PM, Alexander Monakov <amonakov@ispras.ru> wrote:
> > > Declaration of 'ifru_data' in that header has the same issue.
> > 
> > I originally considered changing this one but had left it out since
> > linux disagree on the type (void *).
> 
> Ah. From looking at pre-git Linux history, it appears that it had
> 'char *', but then it was changed to 'void *' in June 2004 by commit
> '[PATCH] sparse: ->ifr_data annotation' by Al Viro, causing a divergence
> with the Glibc interface.
> 
> FreeBSD also has 'char *' (caddr_t) there, and netdevice(7) manpage documents
> that the Linux interface has 'char *' as well. Thus, the kernel uapi header is
> the odd one out.
> 
> Please wait for further feedback from Rich.

I'd lean towards char* because if applications perform arithmetic on
it expecting it to be char*, void* yields UB. The kernel folks love
their void* arithmetic...

Rich

Re: [PATCH] fix if_indextoname error case

[Rich Felker]

On Thu, Sep 15, 2016 at 11:27:30AM -0400, Daniel Sabogal wrote:
> posix requires errno to be set to ENXIO if the interface does not exist.
> linux returns ENODEV instead so we handle this.
> ---
>  src/network/if_indextoname.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

Applied.

Rich