Re: [patch] netcfg get_all_ifs()

[Luca Favatella <slackydeb@gmail.com>]

On 06/07/2009, Frans Pop <elendil@planet.nl> wrote:
> On Monday 06 July 2009, Luca Favatella wrote:
>> This patch substitutes Linux-specific code in get_all_ifs() using
>> getifaddrs().
>
> -        if (!strcmp(ibuf, "lo"))        /* ignore the loopback */
> [...]
> +        if (!strncmp(ibuf, "lo", 2))        /* ignore the loopback */
>
> Please do not include random changes that are unrelated to what you
> document in the changelog in the patch. This _does_ change functionality,
> even if in practice it will not make a difference.
>
> The old code would not match on an (unlikely) interface named "loop", the
> new code will.
>
> Cheers,
> FJP

Solved in the attached version 2 of the patch.

Thanks,
Luca Favatella

Index: debian/changelog
===================================================================
--- debian/changelog	(.../trunk/packages/netcfg)	(revisione 59247)
+++ debian/changelog	(.../branches/d-i/kfreebsd/packages/netcfg)	(revisione 59252)
@@ -8,6 +8,7 @@
   * If wireless is disabled, don't build and link wireless related stuff.
   * Disable by default wireless on non-linux architectures.
   * Inverse the logic about building without wireless support (WIRELESS=0).
+  * Substitute Linux-specific code in get_all_ifs() using getifaddrs().
 
   [ Colin Watson ]
   * check_kill_switch is Linux-specific; provide a stub implementation for
Index: netcfg-common.c
===================================================================
--- netcfg-common.c	(.../trunk/packages/netcfg)	(revisione 59247)
+++ netcfg-common.c	(.../branches/d-i/kfreebsd/packages/netcfg)	(revisione 59252)
@@ -45,6 +45,8 @@
 #include <time.h>
 #include <netdb.h>
 
+#include <ifaddrs.h>
+
 /* Set if there is currently a progress bar displayed. */
 int netcfg_progress_displayed = 0;
 
@@ -227,53 +233,24 @@
     return ((ifr.ifr_flags & IFF_UP) ? 1 : 0);
 }
 
-void get_name(char *name, char *p)
-{
-    while (isspace(*p))
-        p++;
-    while (*p) {
-        if (isspace(*p))
-            break;
-        if (*p == ':') {	/* could be an alias */
-            char *dot = p, *dotname = name;
-            *name++ = *p++;
-            while (isdigit(*p))
-                *name++ = *p++;
-            if (*p != ':') {	/* it wasn't, backup */
-                p = dot;
-                name = dotname;
-            }
-            if (*p == '\0')
-                return;
-            p++;
-            break;
-        }
-        *name++ = *p++;
-    }
-    *name++ = '\0';
-    return;
-}
-
 int get_all_ifs (int all, char*** ptr)
 {
-    FILE *ifs = NULL;
-    char ibuf[512], rbuf[512];
+    struct ifaddrs *ifap, *ifa;
+    char ibuf[512];
     char** list = NULL;
     size_t len = 0;
     
-    if ((ifs = fopen("/proc/net/dev", "r")) != NULL) {
-        fgets(ibuf, sizeof(ibuf), ifs); /* eat header */
-        fgets(ibuf, sizeof(ibuf), ifs); /* ditto */
-    }
-    else
+    if (getifaddrs(&ifap) == -1)
         return 0;
 
-    while (fgets(rbuf, sizeof(rbuf), ifs) != NULL) {
-        get_name(ibuf, rbuf);
+    for (ifa = ifap; ifa; ifa = ifa->ifa_next) {
+        strncpy(ibuf, ifa->ifa_name, sizeof(ibuf));
         if (!strcmp(ibuf, "lo"))        /* ignore the loopback */
             continue;
+#if defined(__linux__)
         if (!strncmp(ibuf, "sit", 3))        /* ignore tunnel devices */
             continue;
+#endif
 #if defined(WIRELESS)
         if (is_raw_80211(ibuf))
             continue;
@@ -290,7 +267,7 @@
         list = realloc(list, sizeof(char*) * (len + 1));
         list[len] = NULL;
     }
-    fclose (ifs);
+    freeifaddrs(ifap);
     
     *ptr = list;