Use FORMAT everywhere, and fix up the errors it finds

Many printf-like variadic functions weren't properly decorated with
FORMAT annotations.  Add them everywhere.

It is not possible to annotate a function pointer type, so that
doesn't work for the function pointers in struct log_if.  However, we
have convenience wrapper functions slilog and vslilog, which are
appropriately decorated and therefore safer.  Change all call sites to
use those instead, and leave a comment.  (Rename the function pointer
variable names so that we don't miss any call sites.)

Fix the bugs that this new compiler checking reveals.  These are
nearly all in fatal error reporting, and not very scary.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>

diff --git a/log.c b/log.c
index d940df530e914c2ffba4b0e6f3c5ad0ffd890668..d330113f0c1b8fe0255eab9ea60f87c37827686c 100644 (file)
--- a/log.c
+++ b/log.c
@@ -14,6 +14,8 @@ bool_t secnet_is_daemon=False;
 uint32_t message_level=M_WARNING|M_ERR|M_SECURITY|M_FATAL;
 struct log_if *system_log=NULL;
 
 uint32_t message_level=M_WARNING|M_ERR|M_SECURITY|M_FATAL;
 struct log_if *system_log=NULL;
 

+static void vMessageFallback(uint32_t class, const char *message, va_list args)
+    FORMAT(printf,2,0);

 static void vMessageFallback(uint32_t class, const char *message, va_list args)
 {
     FILE *dest=stdout;
 static void vMessageFallback(uint32_t class, const char *message, va_list args)
 {
     FILE *dest=stdout;


@@ -60,6 +62,8 @@ void Message(uint32_t class, const char *message, ...)
     va_end(ap);
 }
 
     va_end(ap);
 }
 

+static void MessageFallback(uint32_t class, const char *message, ...)
+    FORMAT(printf,2,3);

 static void MessageFallback(uint32_t class, const char *message, ...)
 {
     va_list ap;
 static void MessageFallback(uint32_t class, const char *message, ...)
 {
     va_list ap;


@@ -191,7 +195,7 @@ static void log_vmulti(void *sst, int class, const char *message, va_list args)
 
     if (secnet_is_daemon) {
        for (i=st; i; i=i->next) {
 
     if (secnet_is_daemon) {
        for (i=st; i; i=i->next) {

-           i->l->vlog(i->l->st,class,message,args);
+           vslilog(i->l,class,message,args);

        }
     } else {
        vMessage(class,message,args);
        }
     } else {
        vMessage(class,message,args);


@@ -199,6 +203,8 @@ static void log_vmulti(void *sst, int class, const char *message, va_list args)
     }
 }
 
     }
 }
 

+static void log_multi(void *st, int priority, const char *message, ...)
+    FORMAT(printf,3,4);

 static void log_multi(void *st, int priority, const char *message, ...)
 {
     va_list ap;
 static void log_multi(void *st, int priority, const char *message, ...)
 {
     va_list ap;


@@ -242,8 +248,8 @@ struct log_if *init_log(list_t *ll)
     }
     r=safe_malloc(sizeof(*r), "init_log");
     r->st=l;
     }
     r=safe_malloc(sizeof(*r), "init_log");
     r->st=l;

-    r->log=log_multi;
-    r->vlog=log_vmulti;
+    r->logfn=log_multi;
+    r->vlogfn=log_vmulti;

     return r;
 }
 
     return r;
 }
 


@@ -283,6 +289,8 @@ static void logfile_vlog(void *sst, int class, const char *message,
     }
 }
 
     }
 }
 

+static void logfile_log(void *state, int class, const char *message, ...)
+    FORMAT(printf,3,4);

 static void logfile_log(void *state, int class, const char *message, ...)
 {
     va_list ap;
 static void logfile_log(void *state, int class, const char *message, ...)
 {
     va_list ap;


@@ -355,8 +363,8 @@ static list_t *logfile_apply(closure_t *self, struct cloc loc, dict_t *context,
     st->cl.apply=NULL;
     st->cl.interface=&st->ops;
     st->ops.st=st;
     st->cl.apply=NULL;
     st->cl.interface=&st->ops;
     st->ops.st=st;

-    st->ops.log=logfile_log;
-    st->ops.vlog=logfile_vlog;
+    st->ops.logfn=logfile_log;
+    st->ops.vlogfn=logfile_vlog;

     st->loc=loc;
     st->f=NULL;
 
     st->loc=loc;
     st->f=NULL;
 


@@ -399,6 +407,9 @@ static int msgclass_to_syslogpriority(uint32_t m)
     }
 }
     
     }
 }
     

+static void syslog_vlog(void *sst, int class, const char *message,
+                        va_list args)
+    FORMAT(printf,3,0);

 static void syslog_vlog(void *sst, int class, const char *message,
                         va_list args)
 {
 static void syslog_vlog(void *sst, int class, const char *message,
                         va_list args)
 {


@@ -412,6 +423,8 @@ static void syslog_vlog(void *sst, int class, const char *message,
     }
 }
 
     }
 }
 

+static void syslog_log(void *sst, int priority, const char *message, ...)
+    FORMAT(printf,3,4);

 static void syslog_log(void *sst, int priority, const char *message, ...)
 {
     va_list ap;
 static void syslog_log(void *sst, int priority, const char *message, ...)
 {
     va_list ap;


@@ -472,8 +485,8 @@ static list_t *syslog_apply(closure_t *self, struct cloc loc, dict_t *context,
     st->cl.apply=NULL;
     st->cl.interface=&st->ops;
     st->ops.st=st;
     st->cl.apply=NULL;
     st->cl.interface=&st->ops;
     st->ops.st=st;

-    st->ops.log=syslog_log;
-    st->ops.vlog=syslog_vlog;
+    st->ops.logfn=syslog_log;
+    st->ops.vlogfn=syslog_vlog;

 
     item=list_elem(args,0);
     if (!item || item->type!=t_dict)
 
     item=list_elem(args,0);
     if (!item || item->type!=t_dict)


@@ -528,7 +541,7 @@ static void log_from_fd_afterpoll(void *sst, struct pollfd *fds, int nfds)
        remain=FDLOG_BUFSIZE-st->i-1;
        if (remain<=0) {
            st->buffer[FDLOG_BUFSIZE-1]=0;
        remain=FDLOG_BUFSIZE-st->i-1;
        if (remain<=0) {
            st->buffer[FDLOG_BUFSIZE-1]=0;

-           st->log->log(st->log,M_WARNING,"%s: overlong line: %s",
+           slilog(st->log,M_WARNING,"%s: overlong line: %s",

                         st->prefix,st->buffer);
            st->i=0;
            remain=FDLOG_BUFSIZE-1;
                         st->prefix,st->buffer);
            st->i=0;
            remain=FDLOG_BUFSIZE-1;


@@ -539,7 +552,7 @@ static void log_from_fd_afterpoll(void *sst, struct pollfd *fds, int nfds)
            for (i=0; i<st->i; i++) {
                if (st->buffer[i]=='\n') {
                    st->buffer[i]=0;
            for (i=0; i<st->i; i++) {
                if (st->buffer[i]=='\n') {
                    st->buffer[i]=0;

-                   st->log->log(st->log->st,M_INFO,"%s: %s",
+                   slilog(st->log,M_INFO,"%s: %s",

                                 st->prefix,st->buffer);
                    i++;
                    memmove(st->buffer,st->buffer+i,st->i-i);
                                 st->prefix,st->buffer);
                    i++;
                    memmove(st->buffer,st->buffer+i,st->i-i);

diff --git a/secnet.c b/secnet.c
index 465a93fc260404cad1eb2070bf8df446013e87a9..c604d44b65781c2cbc7b60604621991cd9e55d32 100644 (file)
--- a/secnet.c
+++ b/secnet.c
@@ -332,7 +332,8 @@ static void run(void)
                fatal("run: beforepoll_fn (%s) returns %d",i->desc,rv);
            }
            if (timeout<-1) {
                fatal("run: beforepoll_fn (%s) returns %d",i->desc,rv);
            }
            if (timeout<-1) {

-               fatal("run: beforepoll_fn (%s) set timeout to %d",timeout);
+               fatal("run: beforepoll_fn (%s) set timeout to %d",
+                     i->desc,timeout);

            }
            idx+=nfds;
            remain-=nfds;
            }
            idx+=nfds;
            remain-=nfds;

diff --git a/secnet.h b/secnet.h
index 252eeb659b7f01fda5a8ba0b61e1788471195249..98a3ae38fa548434fcbf3c0a6a35236c43d8e1cc 100644 (file)
--- a/secnet.h
+++ b/secnet.h
@@ -339,8 +339,8 @@ typedef void log_vmsg_fn(void *st, int class, const char *message,
                         va_list args);
 struct log_if {
     void *st;
                         va_list args);
 struct log_if {
     void *st;

-    log_msg_fn *log;
-    log_vmsg_fn *vlog;
+    log_msg_fn *logfn;   /* Do not call these directly - you don't get */
+    log_vmsg_fn *vlogfn; /* printf format checking.  Use [v]slilog instead */

 };
 /* (convenience functions, defined in util.c) */
 extern void slilog(struct log_if *lf, int class, const char *message, ...)
 };
 /* (convenience functions, defined in util.c) */
 extern void slilog(struct log_if *lf, int class, const char *message, ...)


@@ -492,21 +492,25 @@ struct buffer_if {
 #define M_FATAL               0x100
 
 /* The fatal() family of functions require messages that do not end in '\n' */
 #define M_FATAL               0x100
 
 /* The fatal() family of functions require messages that do not end in '\n' */

-extern NORETURN(fatal(const char *message, ...));
-extern NORETURN(fatal_perror(const char *message, ...));
-extern NORETURN(fatal_status(int status, const char *message, ...));
-extern NORETURN(fatal_perror_status(int status, const char *message, ...));
+extern NORETURN(fatal(const char *message, ...)) FORMAT(printf,1,2);
+extern NORETURN(fatal_perror(const char *message, ...)) FORMAT(printf,1,2);
+extern NORETURN(fatal_status(int status, const char *message, ...))
+       FORMAT(printf,2,3);
+extern NORETURN(fatal_perror_status(int status, const char *message, ...))
+       FORMAT(printf,2,3);

 
 /* The cfgfatal() family of functions require messages that end in '\n' */
 extern NORETURN(cfgfatal(struct cloc loc, cstring_t facility,
 
 /* The cfgfatal() family of functions require messages that end in '\n' */
 extern NORETURN(cfgfatal(struct cloc loc, cstring_t facility,

-                        const char *message, ...));
+                        const char *message, ...)) FORMAT(printf,3,4);

 extern void cfgfile_postreadcheck(struct cloc loc, FILE *f);
 extern NORETURN(vcfgfatal_maybefile(FILE *maybe_f, struct cloc loc,
                                    cstring_t facility, const char *message,
 extern void cfgfile_postreadcheck(struct cloc loc, FILE *f);
 extern NORETURN(vcfgfatal_maybefile(FILE *maybe_f, struct cloc loc,
                                    cstring_t facility, const char *message,

-                                   va_list));
+                                   va_list))
+    FORMAT(printf,4,0);

 extern NORETURN(cfgfatal_maybefile(FILE *maybe_f, struct cloc loc,
                                   cstring_t facility,
 extern NORETURN(cfgfatal_maybefile(FILE *maybe_f, struct cloc loc,
                                   cstring_t facility,

-                                  const char *message, ...));
+                                  const char *message, ...))
+    FORMAT(printf,4,5);

 
 extern void Message(uint32_t class, const char *message, ...)
     FORMAT(printf,2,3);
 
 extern void Message(uint32_t class, const char *message, ...)
     FORMAT(printf,2,3);

diff --git a/site.c b/site.c
index 6c92193e58193e87934e74e8325215d7117974ae..20f9507cee7782c2f801dba295e9a82421855e88 100644 (file)
--- a/site.c
+++ b/site.c
@@ -293,6 +293,8 @@ struct site {
     struct transform_inst_if *new_transform; /* For key setup/verify */
 };
 
     struct transform_inst_if *new_transform; /* For key setup/verify */
 };
 

+static void slog(struct site *st, uint32_t event, cstring_t msg, ...)
+FORMAT(printf,3,4);

 static void slog(struct site *st, uint32_t event, cstring_t msg, ...)
 {
     va_list ap;
 static void slog(struct site *st, uint32_t event, cstring_t msg, ...)
 {
     va_list ap;


@@ -318,7 +320,7 @@ static void slog(struct site *st, uint32_t event, cstring_t msg, ...)
        }
 
        vsnprintf(buf,sizeof(buf),msg,ap);
        }
 
        vsnprintf(buf,sizeof(buf),msg,ap);

-       st->log->log(st->log->st,class,"%s: %s",st->tunname,buf);
+       slilog(st->log,class,"%s: %s",st->tunname,buf);

     }
     va_end(ap);
 }
     }
     va_end(ap);
 }


@@ -1906,7 +1908,7 @@ static bool_t transport_compute_setupinit_peers(struct site *st,
     slog(st,LOG_SETUP_INIT,
         (!configured_addr ? "using only %d old peer address(es)"
          : "using configured address, and/or perhaps %d old peer address(es)"),
     slog(st,LOG_SETUP_INIT,
         (!configured_addr ? "using only %d old peer address(es)"
          : "using configured address, and/or perhaps %d old peer address(es)"),

-        st->peers);
+        st->peers.npeers);

 
     /* Non-mobile peers havve st->peers.npeers==0 or ==1, since they
      * have transport_peers_max==1.  The effect is that this code
 
     /* Non-mobile peers havve st->peers.npeers==0 or ==1, since they
      * have transport_peers_max==1.  The effect is that this code

diff --git a/slip.c b/slip.c
index d8f1a17113f4cd20b8711617f684c07a1f1f159a..0f62a6987268790ce5c4adabf40efa359837a34e 100644 (file)
--- a/slip.c
+++ b/slip.c
@@ -230,7 +230,8 @@ static void userv_userv_callback(void *sst, pid_t pid, int status)
            fatal("%s: userv exited unexpectedly: uncaught signal %d",
                  st->slip.nl.name,WTERMSIG(status));
        } else {
            fatal("%s: userv exited unexpectedly: uncaught signal %d",
                  st->slip.nl.name,WTERMSIG(status));
        } else {

-           fatal("%s: userv stopped unexpectedly");
+           fatal("%s: userv stopped unexpectedly",
+                 st->slip.nl.name);

        }
     }
     Message(M_WARNING,"%s: userv subprocess died with status %d\n",
        }
     }
     Message(M_WARNING,"%s: userv subprocess died with status %d\n",

diff --git a/util.c b/util.c
index cfaefd227148c5f908f707b6f0b31ba48f21037c..24dd9a33ebfb375ea3047425e83b8c8bc6b376c2 100644 (file)
--- a/util.c
+++ b/util.c
@@ -61,7 +61,7 @@ char *safe_strdup(const char *s, const char *message)
     char *d;
     d=strdup(s);
     if (!d) {
     char *d;
     d=strdup(s);
     if (!d) {

-       fatal_perror(message);
+       fatal_perror("%s",message);

     }
     return d;
 }
     }
     return d;
 }


@@ -71,7 +71,7 @@ void *safe_malloc(size_t size, const char *message)
     void *r;
     r=malloc(size);
     if (!r) {
     void *r;
     r=malloc(size);
     if (!r) {

-       fatal_perror(message);
+       fatal_perror("%s",message);

     }
     return r;
 }
     }
     return r;
 }


@@ -208,7 +208,7 @@ bool_t remove_hook(uint32_t phase, hook_fn *fn, void *state)
 
 void vslilog(struct log_if *lf, int priority, const char *message, va_list ap)
 {
 
 void vslilog(struct log_if *lf, int priority, const char *message, va_list ap)
 {

-    lf->vlog(lf->st,priority,message,ap);
+    lf->vlogfn(lf->st,priority,message,ap);

 }
 
 void slilog(struct log_if *lf, int priority, const char *message, ...)
 }
 
 void slilog(struct log_if *lf, int priority, const char *message, ...)