libdbi-devel

[libdbi-devel] dbi_conn_queryf and using __attribute__((format))

[Holger F. <ho...@fr...>]

Hi,

today I used %llu instead of %u and had some runtime error.
GCC can warn about such issues but only if the format attribute
is used[1].

Could you please use this in future releases to catch errors
while compiling the code?

thank you
  holger


[1] http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html

[libdbi-devel] dbi_conn_queryf and using __attribute__((format))

[<mar...@mh...>]

Holger Freyther writes:
 > Hi,
 > 
 > today I used %llu instead of %u and had some runtime error.
 > GCC can warn about such issues but only if the format attribute
 > is used[1].
 > 
 > Could you please use this in future releases to catch errors
 > while compiling the code?
 > 
 > thank you
 >   holger
 > 
 > 
 > [1] http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html
 > 

Hi,

could you please clarify with a code snippet (or better yet, a patch)
what should be changed in the libdbi sources? Also, I take from [1]
that this is a GNU extension of C. Would other compilers like LLVM
complain about the suggested change?

regards,
Markus

-- 
Markus Hoenicka
http://www.mhoenicka.de
AQ score 38 

Re: [libdbi-devel] dbi_conn_queryf and using __attribute__((format))

[Tom L. <tg...@ss...>]

mar...@mh... writes:
> could you please clarify with a code snippet (or better yet, a patch)
> what should be changed in the libdbi sources? Also, I take from [1]
> that this is a GNU extension of C. Would other compilers like LLVM
> complain about the suggested change?

It is a GNU extension, but it's not hard to hide it from non-GNU
compilers.  The Postgres project uses this extensively, and it's caught
many a typo for us.  (Of course, you need to have developers using gcc,
or it doesn't help.  But not everyone has to use it, as long as those
who do use it will notice warnings.)

Some relevant bits from the Postgres sources:

#ifndef __GNUC__
#define __attribute__(_arg_)
#endif

/*
 * Set the format style used by gcc to check printf type functions. We really
 * want the "gnu_printf" style set, which includes what glibc uses, such
 * as %m for error strings and %lld for 64 bit long longs. But not all gcc
 * compilers are known to support it, so we just use "printf" which all
 * gcc versions alive are known to support, except on Windows where
 * using "gnu_printf" style makes a dramatic difference. Maybe someday
 * we'll have a configure test for this, if we ever discover use of more
 * variants to be necessary.
 */
#ifdef WIN32
#define PG_PRINTF_ATTRIBUTE gnu_printf
#else
#define PG_PRINTF_ATTRIBUTE printf
#endif

and here's a typical use:

extern int pg_snprintf(char *str, size_t count, const char *fmt,...)
/* This extension allows gcc to check the format string */
__attribute__((format(PG_PRINTF_ATTRIBUTE, 3, 4)));

Depending on your usage, you might not need the gnu_printf dance.

			regards, tom lane

Re: [libdbi-devel] dbi_conn_queryf and using __attribute__((format))

[<mar...@mh...>]

Tom Lane writes:
 > and here's a typical use:
 > 
 > extern int pg_snprintf(char *str, size_t count, const char *fmt,...)
 > /* This extension allows gcc to check the format string */
 > __attribute__((format(PG_PRINTF_ATTRIBUTE, 3, 4)));
 > 


Hi Tom,

thanks for your explanation. I've checked the sources, and
interestingly src/dbi_main.c contains a declaration of dbi_conn_queryf():

dbi_result dbi_conn_queryf(dbi_conn Conn, const char *formatstr, ...) __attribute__ ((format (printf, 2, 3)));

in addition to the declaration in include/dbi/dbi.h:

dbi_result dbi_conn_queryf(dbi_conn Conn, const char *formatstr, ...);

Was that a half-assed attempt to implement what the OP asked for?
Would it be sufficient to move the first declaration to the header
which is visible to the programs that actually call the function?

regards,
Markus


-- 
Markus Hoenicka
http://www.mhoenicka.de
AQ score 38

Re: [libdbi-devel] dbi_conn_queryf and using __attribute__((format))

[Tom L. <tg...@ss...>]

mar...@mh... writes:
> thanks for your explanation. I've checked the sources, and
> interestingly src/dbi_main.c contains a declaration of dbi_conn_queryf():

> dbi_result dbi_conn_queryf(dbi_conn Conn, const char *formatstr, ...) __attribute__ ((format (printf, 2, 3)));

> in addition to the declaration in include/dbi/dbi.h:

> dbi_result dbi_conn_queryf(dbi_conn Conn, const char *formatstr, ...);

> Was that a half-assed attempt to implement what the OP asked for?
> Would it be sufficient to move the first declaration to the header
> which is visible to the programs that actually call the function?

Right.  The __attribute__() decoration needs to be visible to the call
sites, so generally one sticks it in the header that provides the
function's extern declaration.  There's no point in attaching it to the
function definition.

			regards, tom lane

Re: [libdbi-devel] dbi_conn_queryf and using __attribute__((format))

[<mar...@mh...>]

Tom Lane writes:
 > mar...@mh... writes:
 > > thanks for your explanation. I've checked the sources, and
 > > interestingly src/dbi_main.c contains a declaration of dbi_conn_queryf():
 > 
 > > dbi_result dbi_conn_queryf(dbi_conn Conn, const char *formatstr, ...) __attribute__ ((format (printf, 2, 3)));
 > 
 > > in addition to the declaration in include/dbi/dbi.h:
 > 
 > > dbi_result dbi_conn_queryf(dbi_conn Conn, const char *formatstr, ...);
 > 
 > > Was that a half-assed attempt to implement what the OP asked for?
 > > Would it be sufficient to move the first declaration to the header
 > > which is visible to the programs that actually call the function?
 > 
 > Right.  The __attribute__() decoration needs to be visible to the call
 > sites, so generally one sticks it in the header that provides the
 > function's extern declaration.  There's no point in attaching it to the
 > function definition.
 > 
 > 			regards, tom lane

Thanks Tom. I've checked in the fixed files, see dbi.h.in rev. 1.14
and dbi_main.c rev. 1.104. Holger, could you please verify with your
unintentional testcase whether you get the intended compiler warnings?

regards,
Markus
-- 
Markus Hoenicka
http://www.mhoenicka.de
AQ score 38