Adding format string vs arguments compiler checking to function declarations

Hello, all

Today I have finished several hours long hunting of a very unpleasant bug
which appeared only on the target platform, which had practically no
convenient way to debug my application. The cause appeared to be two cases
of missing arguments for format strings passed to nerr_raise - I have just
forgot to add them.

So I wondered, why the declaration of ClearSilver functions which accept
format strings and arguments do not have corresponding GCC attributes
attached? Of course, these should be applied conditionally, but in the end
it could look like this:

NEOERR *nerr_raisef (const char *func, const char *file, int lineno,
                     NERR_TYPE error, const char *fmt, ...)
                     __attribute__((format(printf, 5, 6)));

As a result the compiler would have warned me and spared me at least 4
continuous hours of slight change/building the whole project/reflashing the
board/booting/testing cycle.

Actually, I have added these attributes to nerr_* functions with format
strings in my copy of ClearSilver. And this had shown several warnings indeed:

neo_files.c: In function `ne_load_file_len':
neo_files.c:94: warning: int format, __off_t arg (arg 6)
csparse.c: In function `eval_expr':
csparse.c:2136: warning: int format, long int arg (arg 7)
csparse.c:2174: warning: int format, long int arg (arg 7)
cgi.c: In function `_parse_post_form':
cgi.c:386: warning: int format, pointer arg (arg 6)

Nothing too serious, though, but nevertheless here is a patch
(inlining it because yahoogroups won't store an attachment):
---:<---
Index: cgi/cgi.c
===================================================================
--- cgi/cgi.c   (revision 41241)
+++ cgi/cgi.c   (working copy)
@@ -384,7 +384,7 @@
   query = (char *) malloc (sizeof(char) * (len + 1));
   if (query == NULL)
     return nerr_raise (NERR_NOMEM,
-       "Unable to allocate memory to read POST input of length %d", l);
+       "Unable to allocate memory to read POST input of length %d", len);


   o = 0;
Index: cs/csparse.c
===================================================================
--- cs/csparse.c        (revision 41241)
+++ cs/csparse.c        (working copy)
@@ -2133,7 +2133,7 @@
           long int n2 = arg_eval_num (parse, &arg2);
           result->s = sprintf_alloc("%s.%d", arg1.s, n2);
           if (result->s == NULL)
-            return nerr_raise (NERR_NOMEM, "Unable to allocate memory to concatenate varnames in expression: %s + %d", arg1.s, n2);
+            return nerr_raise (NERR_NOMEM, "Unable to allocate memory to concatenate varnames in expression: %s + %ld", arg1.s, n2);
         }
         else
         {
@@ -2171,7 +2171,7 @@
             long int n2 = arg_eval_num (parse, &arg2);
             result->s = sprintf_alloc("%s.%d", arg1.s, n2);
             if (result->s == NULL)
-              return nerr_raise (NERR_NOMEM, "Unable to allocate memory to concatenate varnames in expression: %s + %d", arg1.s, n2);
+              return nerr_raise (NERR_NOMEM, "Unable to allocate memory to concatenate varnames in expression: %s + %ld", arg1.s, n2);
           }
           else
           {
Index: util/neo_err.h
===================================================================
--- util/neo_err.h      (revision 41241)
+++ util/neo_err.h      (working copy)
@@ -80,7 +80,8 @@
 #endif

 NEOERR *nerr_raisef (const char *func, const char *file, int lineno,
-                     NERR_TYPE error, const char *fmt, ...);
+                     NERR_TYPE error, const char *fmt, ...)
+                     __attribute__((format(printf, 5, 6)));


 #ifdef __GNUC__
@@ -92,7 +93,9 @@
 #endif

 NEOERR *nerr_raise_errnof (const char *func, const char *file, int lineno,
-                           int error, const char *fmt, ...);
+                           int error, const char *fmt, ...)
+                           __attribute__((format(printf, 5, 6)));
+
 /* function: nerr_pass
  * description: this function is used to pass an error up a level in the
  *              call chain (ie, if the error isn't handled at the
@@ -128,7 +131,8 @@
    nerr_pass_ctxf(__PRETTY_FUNCTION__,__FILE__,__LINE__,e,f,__VA_ARGS__)
 #endif
 NEOERR *nerr_pass_ctxf (const char *func, const char *file, int lineno, NEOERR *err,
-                       const char *fmt, ...);
+                       const char *fmt, ...)
+                       __attribute__((format(printf, 5, 6)));

 /* function: nerr_log_error
  * description: currently, this prints out the error to stderr, and
Index: util/neo_files.c
===================================================================
--- util/neo_files.c    (revision 41241)
+++ util/neo_files.c    (working copy)
@@ -92,7 +92,7 @@
   {
     close(fd);
     return nerr_raise (NERR_NOMEM,
-       "Unable to allocate memory (%d) to load file %s", s.st_size, path);
+       "Unable to allocate memory (%d) to load file %s", len + 1, path);
   }
   if ((bytes_read = read (fd, *str, len)) == -1)
   {
--->:---

Thank you for your attention :)

Sincerely,
Nick



 
Yahoo! Groups Links

<*> To visit your group on the web, go to:
    http://groups.yahoo.com/group/ClearSilver/

<*> Your email settings:
    Individual Email | Traditional

<*> To change settings online go to:
    http://groups.yahoo.com/group/ClearSilver/join
    (Yahoo! ID required)

<*> To change settings via email:
    mailto:[email protected] 
    mailto:[email protected]

<*> To unsubscribe from this group, send an email to:
    [email protected]

<*> Your use of Yahoo! Groups is subject to:
    http://docs.yahoo.com/info/terms/