Patch for network-interface-{list,info}

Patch for network-interface-{list,info}

[Alex Plotnick]

I noticed that the functions `network-interface-info' and
`network-interface-list' did not work on Darwin 6.8 (Mac OS X 10.2.8).
After some investigation, I determined that they probably would not work on
some other BSDs as well.  The following patch addresses some of the issues.

The use of ioctl(SIOCGIFCONF) to fetch the list of network interfaces is
prone to some interesting problems.  In particular, the size of the
returned structures (struct ifreq) is not constant on some systems (e.g.,
Darwin), making the buffer resizing and list iteration somewhat tricky.  My
patch fixes these errors, and the SIOCGIFCONF now works properly under both
GNU/Linux and Darwin.  However, it seems to be preferable to just use
getifaddrs() when available, as it's a much cleaner interface.  To my mind,
it's tempting to get rid of the SIOCGIFCONF code all together, but I've
left it in for now.

In the case of `network-interface-info', certain of the requested ioctl()s
are non-existent on Darwin (and, again, presumably on some other BSDs); in
particular, SIOCGIFNETMASK and SIOCGIFHWADDR.  I've got some code that
uses getifaddrs() to fetch all of this info (instead of doing separate
ioctl() calls), but have not included it in this patch.  If people think
this is the correct way to go, I'll send it on.  (For those that care: the
downside of this is the difference between the link-layer sockaddrs on BSD
(struct sockaddr_dl) and Linux (struct sockaddr_ll).)

Comments, suggestions, bug-fixes, and test reports are all welcome.

          -- Alex Plotnick


Index: configure.in
===================================================================
RCS file: /cvsroot/emacs/emacs/configure.in,v
retrieving revision 1.356
diff -u -r1.356 configure.in
--- configure.in	23 Sep 2003 12:39:44 -0000	1.356
+++ configure.in	16 Nov 2003 20:05:24 -0000
@@ -1516,7 +1516,9 @@
 		[#include <time.h>])
 AC_CHECK_MEMBERS([struct ifreq.ifr_flags, struct ifreq.ifr_hwaddr, 
 		  struct ifreq.ifr_netmask, struct ifreq.ifr_broadaddr,
-		  struct ifreq.ifr_addr], , ,[#include <net/if.h>])
+		  struct ifreq.ifr_addr], , ,[#include <sys/types.h>
+#include <sys/socket.h>
+#include <net/if.h>])
 
 dnl checks for compiler characteristics
 
@@ -2298,7 +2300,7 @@
 rename closedir mkdir rmdir sysinfo \
 random lrand48 bcopy bcmp logb frexp fmod rint cbrt ftime res_init setsid \
 strerror fpathconf select mktime euidaccess getpagesize tzset setlocale \
-utimes setrlimit setpgid getcwd getwd shutdown getaddrinfo \
+utimes setrlimit setpgid getcwd getwd shutdown getaddrinfo getifaddrs \
 __fpending mblen mbrlen mbsinit strsignal setitimer ualarm index rindex \
 sendto recvfrom getsockopt setsockopt getsockname getpeername \
 gai_strerror mkstemp getline getdelim mremap memmove fsync bzero \
Index: src/process.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/process.c,v
retrieving revision 1.416
diff -u -r1.416 process.c
--- src/process.c	27 Sep 2003 00:25:40 -0000	1.416
+++ src/process.c	16 Nov 2003 20:05:26 -0000
@@ -109,6 +109,10 @@
 #endif
 #endif
 
+#ifdef HAVE_GETIFADDRS
+#include <ifaddrs.h>
+#endif
+
 #ifdef IRIS
 #include <sys/sysmacros.h>	/* for "minor" */
 #endif /* not IRIS */
@@ -3353,7 +3357,7 @@
 \f
 #if defined(HAVE_SOCKETS) && defined(HAVE_NET_IF_H) && defined(HAVE_SYS_IOCTL_H)
 
-#ifdef SIOCGIFCONF
+#if defined(HAVE_GETIFADDRS) || defined(SIOCGIFCONF)
 DEFUN ("network-interface-list", Fnetwork_interface_list, Snetwork_interface_list, 0, 0, 0,
        doc: /* Return an alist of all network interfaces and their network address.
 Each element is a cons, the car of which is a string containing the
@@ -3361,58 +3365,89 @@
 format; see the description of ADDRESS in `make-network-process'.  */)
      ()
 {
+  Lisp_Object res = Qnil;
+#ifdef HAVE_GETIFADDRS
+  struct ifaddrs *ifap, *ifa;
+
+  if (getifaddrs (&ifap) != 0)
+      return Qnil;
+
+  for (ifa = ifap; ifa != NULL; ifa = ifa->ifa_next)
+    {
+      char namebuf[sizeof (ifa->ifa_name) + 1];
+      if (ifa->ifa_addr->sa_family != AF_INET)
+	continue;
+      bcopy (ifa->ifa_name, namebuf, sizeof (ifa->ifa_name));
+      namebuf[sizeof (ifa->ifa_name)] = 0;
+      res = Fcons (Fcons (build_string (namebuf),
+			  conv_sockaddr_to_lisp (ifa->ifa_addr,
+						sizeof (struct sockaddr))),
+		   res);
+    }
+  freeifaddrs(ifap);
+
+#else /* defined(SIOCGIFCONF) */
   struct ifconf ifconf;
-  struct ifreq *ifreqs = NULL;
-  int ifaces = 0;
+  struct ifreq *ifrp, *ifend, *ifnext;
+  char *buf = NULL;
   int buf_size, s;
-  Lisp_Object res;
 
-  s = socket (AF_INET, SOCK_STREAM, 0);
+  s = socket (AF_INET, SOCK_DGRAM, 0);
   if (s < 0)
     return Qnil;
 
+  buf_size = 0;
  again:
-  ifaces += 25;
-  buf_size = ifaces * sizeof(ifreqs[0]);
-  ifreqs = (struct ifreq *)xrealloc(ifreqs, buf_size);
-  if (!ifreqs)
+  buf_size += 1024;
+  buf = (char *) xrealloc (buf, buf_size);
+  if (!buf)
     {
       close (s);
       return Qnil;
     }
 
   ifconf.ifc_len = buf_size;
-  ifconf.ifc_req = ifreqs;
-  if (ioctl (s, SIOCGIFCONF, &ifconf))
+  ifconf.ifc_buf = buf;
+  bzero (buf, buf_size);
+  if (ioctl (s, SIOCGIFCONF, &ifconf) == -1)
     {
       close (s);
+      xfree (buf);
       return Qnil;
     }
 
-  if (ifconf.ifc_len == buf_size)
+  if (buf_size - ifconf.ifc_len < sizeof (struct ifreq))
     goto again;
 
   close (s);
-  ifaces = ifconf.ifc_len / sizeof (ifreqs[0]);
 
-  res = Qnil;
-  while (--ifaces >= 0)
+#ifdef _SIZEOF_ADDR_IFREQ
+#define sizeof_ifreq(IFREQ) _SIZEOF_ADDR_IFREQ (IFREQ)
+#else
+#define sizeof_ifreq(IFREQ) sizeof (struct ifreq)
+#endif
+
+  ifrp = (struct ifreq *) buf;
+  ifend = (struct ifreq *)(buf + ifconf.ifc_len);
+  for ( ; ifrp < ifend; ifrp = ifnext)
     {
-      struct ifreq *ifq = &ifreqs[ifaces];
-      char namebuf[sizeof (ifq->ifr_name) + 1];
-      if (ifq->ifr_addr.sa_family != AF_INET)
+      ifnext = (struct ifreq *) ((char *) ifrp + sizeof_ifreq (*ifrp));
+      char namebuf[sizeof (ifrp->ifr_name) + 1];
+      if (ifrp->ifr_addr.sa_family != AF_INET)
 	continue;
-      bcopy (ifq->ifr_name, namebuf, sizeof (ifq->ifr_name));
-      namebuf[sizeof (ifq->ifr_name)] = 0;
+      bcopy (ifrp->ifr_name, namebuf, sizeof (ifrp->ifr_name));
+      namebuf[sizeof (ifrp->ifr_name)] = 0;
       res = Fcons (Fcons (build_string (namebuf),
-			  conv_sockaddr_to_lisp (&ifq->ifr_addr,
+			  conv_sockaddr_to_lisp (&ifrp->ifr_addr,
 						 sizeof (struct sockaddr))),
 		   res);
     }
+  xfree (buf);
+#endif /* SIOCGIFCONF */
 
   return res;
 }
-#endif /* SIOCGIFCONF */
+#endif /* defined(HAVE_GETIFADDRS) || defined(SIOCGIFCONF) */
 
 #if defined(SIOCGIFADDR) || defined(SIOCGIFHWADDR) || defined(SIOCGIFFLAGS)
 
@@ -3473,6 +3508,29 @@
   { 0, 0 }
 };
 
+static Lisp_Object
+conv_ifflags_to_lisp (flags)
+     short int flags;
+{
+  Lisp_Object obj = Qnil;
+  struct ifflag_def *fp;
+  int fnum;
+
+  for (fp = ifflag_table; fp->flag_bit && fp->flag_sym; fp++)
+    {
+      if (flags & fp->flag_bit)
+	{
+	  obj = Fcons (intern (fp->flag_sym), obj);
+	  flags -= fp->flag_bit;
+	}
+    }
+  for (fnum = 0; flags && fnum < 16; fnum++)
+      if (flags & (1 << fnum))
+	obj = Fcons (make_number (fnum), obj);
+
+  return obj;
+}
+
 DEFUN ("network-interface-info", Fnetwork_interface_info, Snetwork_interface_info, 1, 1, 0,
        doc: /* Return information about network interface named IFNAME.
 The return value is a list (ADDR BCAST NETMASK HWADDR FLAGS),
@@ -3499,35 +3557,17 @@
 
   elt = Qnil;
 #if defined(SIOCGIFFLAGS) && defined(HAVE_STRUCT_IFREQ_IFR_FLAGS)
-  if (ioctl (s, SIOCGIFFLAGS, &rq) == 0)
+  if (ioctl (s, SIOCGIFFLAGS, &rq) != -1)
     {
-      int flags = rq.ifr_flags;
-      struct ifflag_def *fp;
-      int fnum;
-
       any++;
-      for (fp = ifflag_table; flags != 0 && fp; fp++)
-	{
-	  if (flags & fp->flag_bit)
-	    {
-	      elt = Fcons (intern (fp->flag_sym), elt);
-	      flags -= fp->flag_bit;
-	    }
-	}
-      for (fnum = 0; flags && fnum < 32; fnum++)
-	{
-	  if (flags & (1 << fnum))
-	    {
-	      elt = Fcons (make_number (fnum), elt);
-	    }
-	}
+      elt = conv_ifflags_to_lisp (rq.ifr_flags);
     }
 #endif
   res = Fcons (elt, res);
 
   elt = Qnil;
 #if defined(SIOCGIFHWADDR) && defined(HAVE_STRUCT_IFREQ_IFR_HWADDR)
-  if (ioctl (s, SIOCGIFHWADDR, &rq) == 0)
+  if (ioctl (s, SIOCGIFHWADDR, &rq) != -1)
     {
       Lisp_Object hwaddr = Fmake_vector (make_number (6), Qnil);
       register struct Lisp_Vector *p = XVECTOR (hwaddr);
@@ -3543,7 +3583,7 @@
 
   elt = Qnil;
 #if defined(SIOCGIFNETMASK) && defined(ifr_netmask)
-  if (ioctl (s, SIOCGIFNETMASK, &rq) == 0)
+  if (ioctl (s, SIOCGIFNETMASK, &rq) != -1)
     {
       any++;
       elt = conv_sockaddr_to_lisp (&rq.ifr_netmask, sizeof (rq.ifr_netmask));
@@ -3553,7 +3593,7 @@
 
   elt = Qnil;
 #if defined(SIOCGIFBRDADDR) && defined(HAVE_STRUCT_IFREQ_IFR_BROADADDR)
-  if (ioctl (s, SIOCGIFBRDADDR, &rq) == 0)
+  if (ioctl (s, SIOCGIFBRDADDR, &rq) != -1)
     {
       any++;
       elt = conv_sockaddr_to_lisp (&rq.ifr_broadaddr, sizeof (rq.ifr_broadaddr));
@@ -3563,7 +3603,7 @@
 
   elt = Qnil;
 #if defined(SIOCGIFADDR) && defined(HAVE_STRUCT_IFREQ_IFR_ADDR)
-  if (ioctl (s, SIOCGIFADDR, &rq) == 0)
+  if (ioctl (s, SIOCGIFADDR, &rq) != -1)
     {
       any++;
       elt = conv_sockaddr_to_lisp (&rq.ifr_addr, sizeof (rq.ifr_addr));

Re: Patch for network-interface-{list,info}

[Kim F. Storm]

[To RMS:  We need papers from Alex to use his patches]

Alex Plotnick <shrike@netaxs.com> writes:

> I noticed that the functions `network-interface-info' and
> `network-interface-list' did not work on Darwin 6.8 (Mac OS X 10.2.8).
> After some investigation, I determined that they probably would not work on
> some other BSDs as well.  The following patch addresses some of the issues.
> 
> The use of ioctl(SIOCGIFCONF) to fetch the list of network interfaces is
> prone to some interesting problems.  In particular, the size of the
> returned structures (struct ifreq) is not constant on some systems (e.g.,
> Darwin), making the buffer resizing and list iteration somewhat tricky.  

Ah, yes, I'd forgotten all about the sa_len issue...  Thanks for
pointing that out.

>                                                                          My
> patch fixes these errors, and the SIOCGIFCONF now works properly under both
> GNU/Linux and Darwin.  However, it seems to be preferable to just use
> getifaddrs() when available, as it's a much cleaner interface.  To my mind,
> it's tempting to get rid of the SIOCGIFCONF code all together, but I've
> left it in for now.

It's definitely a cleaner interface, yes, and maybe it is as widely
available as SIOCGIFCONF these days...?

But in any case, since network-interface-list is a new addition in CVS
emacs, I think it would be alright for us to restrict its availability
to those systems which do support getifaddrs (in some form -- AFAIK,
there are variations in the amount of useful information returned by
getifaddrs on different systems [and I have seen bug reports about
traps in getifaddrs if there are many interfaces]).

If we settle on using only getifaddrs, I would suggest that we change
network-interface-list to return all the available information about
each interface (based on the information returned by getifaddrs),
similar to what network-interface-info return now, and just let
network-interface-info do something like

(defun network-interface-info (INTERFACE)
  (assoc-default INTERFACE (network-interface-list)))

> 
> In the case of `network-interface-info', certain of the requested ioctl()s
> are non-existent on Darwin (and, again, presumably on some other BSDs); in
> particular, SIOCGIFNETMASK and SIOCGIFHWADDR.  I've got some code that
> uses getifaddrs() to fetch all of this info (instead of doing separate
> ioctl() calls), but have not included it in this patch.  If people think
> this is the correct way to go, I'll send it on.  (For those that care: the
> downside of this is the difference between the link-layer sockaddrs on BSD
> (struct sockaddr_dl) and Linux (struct sockaddr_ll).)

This is unclear to me.

How does getifaddrs return the hw address?  In ifaddr->ifa_data ?
Is that portable?  If not, maybe it could use SIOCGIFHWADDR as 
a fallback.

I haven't found any docs about the value of ifa_data on GNU/Linux.
Do you have ref. pointers for this?

> 
> Comments, suggestions, bug-fixes, and test reports are all welcome.

If you want to modify network-interface-list and -info according to
the suggestions above, I would appreciate it.  

However, AFAICS, you have not signed papers for contributing to emacs,
and your changes are a little too big to be considered a tiny change.
Are you interested in signing papers for your changes?

Otherwise, I will work on this myself.  Thank you for your input.


-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Patch for network-interface-{list,info}

[Alex Plotnick]

storm@cua.dk (Kim F. Storm) writes:

> It's definitely a cleaner interface, yes, and maybe it is as widely
> available as SIOCGIFCONF these days...?

I agree that it's much cleaner.  As far as I know, the free BSDs all have
it, and it's in glibc on GNU/Linux.  Irix doesn't have it; I don't know
about Solaris.

> But in any case, since network-interface-list is a new addition in CVS
> emacs, I think it would be alright for us to restrict its availability
> to those systems which do support getifaddrs.

This seems reasonable to me.  If anyone knows of a good reason why this is
not an acceptable course of action, please let me know.

> How does getifaddrs return the hw address?  In ifaddr->ifa_data ?
> Is that portable?  If not, maybe it could use SIOCGIFHWADDR as 
> a fallback.

getifaddrs() actually returns *two* entries for each interface: one
describes the layer-3 address, and the other the link-layer address.  Which
one is which is determined by examining the sa_family of the given sockaddr
(ifa_addr).  On BSD, this sockaddr is actually a struct sockaddr_dl and has
family AF_LINK, whereas on GNU/Linux it's an AF_PACKET and is described by
a struct sockaddr_ll.  There's no need to muck around with the raw ifa_data
field.

> If you want to modify network-interface-list and -info according to
> the suggestions above, I would appreciate it.

I'll send a patch in the next day or two.

> Are you interested in signing papers for your changes?

This is no problem.  If the relevant party contacts me privately via email,
I'll be happy to provide a mailing address for whatever paperwork is
necessary.

Thanks,

        Alex Plotnick

Re: Patch for network-interface-{list,info}

[Kim F. Storm]

Alex Plotnick <shrike@netaxs.com> writes:

> storm@cua.dk (Kim F. Storm) writes:
> 
> > It's definitely a cleaner interface, yes, and maybe it is as widely
> > available as SIOCGIFCONF these days...?
> 
> I agree that it's much cleaner.  As far as I know, the free BSDs all have
> it, and it's in glibc on GNU/Linux.  Irix doesn't have it; I don't know
> about Solaris.
> 
> > But in any case, since network-interface-list is a new addition in CVS
> > emacs, I think it would be alright for us to restrict its availability
> > to those systems which do support getifaddrs.
> 
> This seems reasonable to me.  If anyone knows of a good reason why this is
> not an acceptable course of action, please let me know.

We really want emacs to be as portable as possible, and that includes
the Emacs Lisp code.  So if you say that getifaddrs is not really as
widely available as SIOCGIFCONF -- and getifaddrs has its own problems
with portability (as you indicate below) -- I think we should keep
SIOCGIFCONF as a fallback -- at least for the moment.

Of course, the version based on SIOCGIFCONF will also have to be
reworked to return the additional information like the getifaddrs
version will do.

> 
> > How does getifaddrs return the hw address?  In ifaddr->ifa_data ?
> > Is that portable?  If not, maybe it could use SIOCGIFHWADDR as 
> > a fallback.
> 
> getifaddrs() actually returns *two* entries for each interface: one
> describes the layer-3 address, and the other the link-layer address.  Which
> one is which is determined by examining the sa_family of the given sockaddr
> (ifa_addr).  On BSD, this sockaddr is actually a struct sockaddr_dl and has
> family AF_LINK, whereas on GNU/Linux it's an AF_PACKET and is described by
> a struct sockaddr_ll.  There's no need to muck around with the raw ifa_data
> field.

This doesn't sound very portable to me.  Is there any way to know how
a given getifaddrs behave in this respect?

> 
> > If you want to modify network-interface-list and -info according to
> > the suggestions above, I would appreciate it.
> 
> I'll send a patch in the next day or two.

Good, thanks.

> 
> > Are you interested in signing papers for your changes?
> 
> This is no problem.  If the relevant party contacts me privately via email,
> I'll be happy to provide a mailing address for whatever paperwork is
> necessary.

Richard?

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk