Re: Adding format string vs arguments compiler checking to function declarations

Cool, yeah.  I've made the change, and added it to the various other
printf style functions, and caught a couple other minor things.

It'll be in the next version.

Brandon

On 07/03/07 Nikolai Kondrashov uttered the following other thing:
> 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
> 
> 
> 

-- 
   "... but a SubGenius with Pink sex slaves has mucho Slack."
                                     -- Giles Bowkett
                                           http://www.fiction.net/blong/