[Qemu-devel] Add argument checking for a number of functions

[Qemu-devel] Add argument checking for a number of functions

[Stefan Weil]

gcc can check printf like variable arguments.
These patches tell gcc to do so for several functions.

[PATCH 1/4] blockdev.h: Add GCC attribute (check format arguments)
[PATCH 2/4] darwin-user: Add GCC attribute (check format arguments)
[PATCH 3/4] qemu-char.h: Add GCC attribute (check format arguments)
[PATCH 4/4] slirp.h: Add GCC attribute (check format arguments)

[Qemu-devel] [PATCH 1/4] blockdev.h: Add GCC attribute (check format arguments)

[Stefan Weil]

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 blockdev.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/blockdev.h b/blockdev.h
index 23ea576..3c5c85d 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -42,7 +42,8 @@ extern int drive_get_max_bus(BlockInterfaceType type);
 extern void drive_uninit(DriveInfo *dinfo);
 extern const char *drive_get_serial(BlockDriverState *bdrv);
 
-extern QemuOpts *drive_add(const char *file, const char *fmt, ...);
+extern QemuOpts *drive_add(const char *file, const char *fmt, ...)
+    __attribute__ ((__format__ (__printf__, 2, 3)));
 extern DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi,
                              int *fatal_error);
 
-- 
1.7.1

Re: [Qemu-devel] [PATCH 1/4] blockdev.h: Add GCC attribute (check format arguments)

[Blue Swirl]

On Thu, Jul 1, 2010 at 11:08 AM, Stefan Weil <weil@mail.berlios.de> wrote:
> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
> ---
>  blockdev.h |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/blockdev.h b/blockdev.h
> index 23ea576..3c5c85d 100644
> --- a/blockdev.h
> +++ b/blockdev.h
> @@ -42,7 +42,8 @@ extern int drive_get_max_bus(BlockInterfaceType type);
>  extern void drive_uninit(DriveInfo *dinfo);
>  extern const char *drive_get_serial(BlockDriverState *bdrv);
>
> -extern QemuOpts *drive_add(const char *file, const char *fmt, ...);
> +extern QemuOpts *drive_add(const char *file, const char *fmt, ...)
> +    __attribute__ ((__format__ (__printf__, 2, 3)));
>  extern DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi,
>                              int *fatal_error);

I lost the cover letter, so this applies to all patches: Wouldn't it
make sense to make GCC_FMT_ATTR(n, m) from audio/audio_int.h available
universally and then use that?

Re: [Qemu-devel] [PATCH 1/4] blockdev.h: Add GCC attribute (check format arguments)

[Stefan Weil]

Am 01.07.2010 22:10, schrieb Blue Swirl:
> On Thu, Jul 1, 2010 at 11:08 AM, Stefan Weil <weil@mail.berlios.de> wrote:
>> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
>> ---
>>  blockdev.h |    3 ++-
>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/blockdev.h b/blockdev.h
>> index 23ea576..3c5c85d 100644
>> --- a/blockdev.h
>> +++ b/blockdev.h
>> @@ -42,7 +42,8 @@ extern int drive_get_max_bus(BlockInterfaceType type);
>>  extern void drive_uninit(DriveInfo *dinfo);
>>  extern const char *drive_get_serial(BlockDriverState *bdrv);
>>
>> -extern QemuOpts *drive_add(const char *file, const char *fmt, ...);
>> +extern QemuOpts *drive_add(const char *file, const char *fmt, ...)
>> +    __attribute__ ((__format__ (__printf__, 2, 3)));
>>  extern DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi,
>>                              int *fatal_error);
>
> I lost the cover letter, so this applies to all patches: Wouldn't it
> make sense to make GCC_FMT_ATTR(n, m) from audio/audio_int.h available
> universally and then use that?


That's a matter of personal taste:

GCC_FMT_ATTR(n, m) is shorter, but a human reader has to
look it up once to see what it does (ok, some readers might
guess it right). The compiler has to look it up, too, so
a common header file is needed.

When I prepared the patches, I did not notice that some
functions used GCC_FMT_ATTR. I added the __attribute__
macro to these functions and got a compiler error...
This shows that at least for me GCC_FMT_ATTR was confusing
(of course it no longer is).

Some people prefer GCC_FMT_ATTR because they want to
be able to redefine it for non-gcc compilers.

__attribute__ can also be redefined for that case, so that
is not a very strong argument.

I prefer using __attribute__ without intermediate macro,
but don't mind if a different style is preferred for qemu.

Regards
Stefan

Re: [Qemu-devel] [PATCH 1/4] blockdev.h: Add GCC attribute (check format arguments)

[malc]

On Thu, 1 Jul 2010, Stefan Weil wrote:

> Am 01.07.2010 22:10, schrieb Blue Swirl:
> > On Thu, Jul 1, 2010 at 11:08 AM, Stefan Weil <weil@mail.berlios.de> wrote:
> > > Signed-off-by: Stefan Weil <weil@mail.berlios.de>
> > > ---
> > >  blockdev.h |    3 ++-
> > >  1 files changed, 2 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/blockdev.h b/blockdev.h
> > > index 23ea576..3c5c85d 100644
> > > --- a/blockdev.h
> > > +++ b/blockdev.h
> > > @@ -42,7 +42,8 @@ extern int drive_get_max_bus(BlockInterfaceType type);
> > >  extern void drive_uninit(DriveInfo *dinfo);
> > >  extern const char *drive_get_serial(BlockDriverState *bdrv);
> > > 
> > > -extern QemuOpts *drive_add(const char *file, const char *fmt, ...);
> > > +extern QemuOpts *drive_add(const char *file, const char *fmt, ...)
> > > +    __attribute__ ((__format__ (__printf__, 2, 3)));
> > >  extern DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi,
> > >                              int *fatal_error);
> > 
> > I lost the cover letter, so this applies to all patches: Wouldn't it
> > make sense to make GCC_FMT_ATTR(n, m) from audio/audio_int.h available
> > universally and then use that?
> 
> 
> That's a matter of personal taste:
> 
> GCC_FMT_ATTR(n, m) is shorter, but a human reader has to
> look it up once to see what it does (ok, some readers might
> guess it right). The compiler has to look it up, too, so
> a common header file is needed.
> 
> When I prepared the patches, I did not notice that some
> functions used GCC_FMT_ATTR. I added the __attribute__
> macro to these functions and got a compiler error...
> This shows that at least for me GCC_FMT_ATTR was confusing
> (of course it no longer is).
> 
> Some people prefer GCC_FMT_ATTR because they want to
> be able to redefine it for non-gcc compilers.
> 
> __attribute__ can also be redefined for that case, so that
> is not a very strong argument.

Just to be pedantic, you can _not_ redfined __attribute the
name is reserved..

> 
> I prefer using __attribute__ without intermediate macro,
> but don't mind if a different style is preferred for qemu.
> 
> Regards
> Stefan
> 
> 

-- 
mailto:av1474@comtv.ru

[Qemu-devel] [PATCH 2/4] darwin-user: Add GCC attribute (check format arguments)

[Stefan Weil]

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 darwin-user/qemu.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/darwin-user/qemu.h b/darwin-user/qemu.h
index 462bbda..c8adce7 100644
--- a/darwin-user/qemu.h
+++ b/darwin-user/qemu.h
@@ -100,7 +100,7 @@ int do_sigaction(int sig, const struct sigaction *act,
 int do_sigaltstack(const struct sigaltstack *ss, struct sigaltstack *oss);
 
 void gemu_log(const char *fmt, ...) __attribute__((format(printf,1,2)));
-void qerror(const char *fmt, ...);
+void qerror(const char *fmt, ...) __attribute__((format(printf, 1, 2)));
 
 void write_dt(void *ptr, unsigned long addr, unsigned long limit, int flags);
 
-- 
1.7.1

[Qemu-devel] [PATCH 3/4] qemu-char.h: Add GCC attribute (check format arguments)

[Stefan Weil]

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 qemu-char.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/qemu-char.h b/qemu-char.h
index e3a0783..1bbac6e 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -76,7 +76,8 @@ CharDriverState *qemu_chr_open_opts(QemuOpts *opts,
                                     void (*init)(struct CharDriverState *s));
 CharDriverState *qemu_chr_open(const char *label, const char *filename, void (*init)(struct CharDriverState *s));
 void qemu_chr_close(CharDriverState *chr);
-void qemu_chr_printf(CharDriverState *s, const char *fmt, ...);
+void qemu_chr_printf(CharDriverState *s, const char *fmt, ...)
+    __attribute__ ((__format__ (__printf__, 2, 3)));
 int qemu_chr_write(CharDriverState *s, const uint8_t *buf, int len);
 void qemu_chr_send_event(CharDriverState *s, int event);
 void qemu_chr_add_handlers(CharDriverState *s,
-- 
1.7.1

[Qemu-devel] [PATCH 4/4] slirp.h: Add GCC attribute (check format arguments)

[Stefan Weil]

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 slirp/slirp.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/slirp/slirp.h b/slirp/slirp.h
index 98a2644..c30c19e 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -294,7 +294,7 @@ void if_start(struct ttys *);
  long gethostid(void);
 #endif
 
-void lprint(const char *, ...);
+void lprint(const char *, ...) __attribute__ ((__format__ (__printf__, 1, 2)));
 
 #ifndef _WIN32
 #include <netdb.h>
-- 
1.7.1

Re: [Qemu-devel] Add argument checking for a number of functions

[Richard Henderson]

On 07/01/2010 04:08 AM, Stefan Weil wrote:
> gcc can check printf like variable arguments.
> These patches tell gcc to do so for several functions.
> 
> [PATCH 1/4] blockdev.h: Add GCC attribute (check format arguments)
> [PATCH 2/4] darwin-user: Add GCC attribute (check format arguments)
> [PATCH 3/4] qemu-char.h: Add GCC attribute (check format arguments)
> [PATCH 4/4] slirp.h: Add GCC attribute (check format arguments)

Acked-by: Richard Henderson <rth@redhat.com>


r~