Re: [PATCH] cleaning ifwatchd

[Roy Marples <roy%marples.name@localhost>]

On Fri, 2008-09-05 at 10:32 +0100, Roy Marples wrote:
> On Fri, 2008-09-05 at 11:17 +0200, Martin Husemann wrote:
> > On Fri, Sep 05, 2008 at 10:15:01AM +0100, Roy Marples wrote:
> > > %m is a glibc extension.
> > > http://www.opengroup.org/onlinepubs/000095399/functions/printf.html
> > > And the NetBSD man page for printf do not document %m.
> > 
> > I mean we are talking about syslog(3), not printf - so it's obviously
> > supporting %m. For a plain printf call the warning would be ok, but this
> > is probably just "not good enough __attribute()__ magic".
> 
> Hmmm, you're right. I didn't spot that in the syslog man page.
> I lack __attribute__ foo to fix it though.

Just following up with this.
It seems that the error is there when using -pedantic
http://gcc.gnu.org/ml/gcc/2002-09/msg00428.html
Here's a proposed syntax that was never followed up - pity
http://gcc.gnu.org/ml/gcc/2002-09/msg00604.html

OK, so new patch attached. Please review :)

I've been patching dhcpcd to use %m instead of strerror as the logging
function does use syslog - but it also outputs to stdout if not
daemonised. For this I've added a conversion routine for libc's whose
printf does not support %m. This has knocked about 300 bytes off the
final size as reported by size and a whole 60 bytes off the file size
but does have the extra benefit of making the code more readable.

Thanks

Roy

Index: ifwatchd.c
===================================================================
RCS file: /cvsroot/src/usr.sbin/ifwatchd/ifwatchd.c,v
retrieving revision 1.23
diff -u -p -r1.23 ifwatchd.c
--- ifwatchd.c  24 May 2008 17:45:14 -0000      1.23
+++ ifwatchd.c  5 Sep 2008 08:37:10 -0000
@@ -239,7 +241,8 @@ usage(void)
            "\t -A <cmd> specify command to run on interface arrival event\n"
            "\t -c <cmd> specify command to run on interface carrier-detect 
event\n"
            "\t -D <cmd> specify command to run on interface departure event\n"
-           "\t -d <cmd> specify command to run on interface down event\n"
+           "\t -d <cmd> specify command to run on interface down event\n");
+       fprintf(stderr,
            "\t -n <cmd> specify command to run on interface no-carrier-detect 
event\n"
            "\t -h       show this help message\n"
            "\t -i       no (!) initial run of the up script if the interface\n"
@@ -257,6 +260,7 @@ dispatch(void *msg, size_t len)
        struct if_msghdr *ifmp;
        struct ifa_msghdr *ifam;
        enum event ev;
+       len = 1;
 
        switch (hd->rtm_type) {
        case RTM_NEWADDR:
@@ -524,12 +529,12 @@ free_interfaces(void)
 }
 
 static int
-find_interface(int index)
+find_interface(int idx)
 {
        struct interface_data * p;
 
        SLIST_FOREACH(p, &ifs, next)
-               if (p->index == index)
+               if (p->index == idx)
                        return 1;
        return 0;
 }
@@ -602,7 +607,7 @@ out:
 static int
 check_is_connected(const char *ifname, int def_retval)
 {
-       int s, err;
+       int s, errn;
        struct spppstatus oldstatus;
        struct spppstatusncp status;
 
@@ -614,10 +619,10 @@ check_is_connected(const char *ifname, i
        s = socket(AF_INET, SOCK_DGRAM, 0);
        if (s < 0)
                return 1;       /* no idea how to handle this... */
-       err = ioctl(s, SPPPGETSTATUSNCP, &status);
-       if (err != 0) {
-               err = ioctl(s, SPPPGETSTATUS, &oldstatus);
-               if (err != 0) {
+       errn = ioctl(s, SPPPGETSTATUSNCP, &status);
+       if (errn != 0) {
+               errn = ioctl(s, SPPPGETSTATUS, &oldstatus);
+               if (errn != 0) {
                        /* not if_spppsubr.c based - return default */
                        close(s);
                        return def_retval;