From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=41223 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OURB9-000657-52 for qemu-devel@nongnu.org; Thu, 01 Jul 2010 17:20:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OURB3-0007Gi-UI for qemu-devel@nongnu.org; Thu, 01 Jul 2010 17:20:14 -0400 Received: from moutng.kundenserver.de ([212.227.17.9]:58962) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OURB3-0007GL-Ih for qemu-devel@nongnu.org; Thu, 01 Jul 2010 17:20:09 -0400 Message-ID: <4C2D0685.9050403@mail.berlios.de> Date: Thu, 01 Jul 2010 23:20:05 +0200 From: Stefan Weil MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 1/4] blockdev.h: Add GCC attribute (check format arguments) References: <1277982502-4196-1-git-send-email-weil@mail.berlios.de> <1277982502-4196-2-git-send-email-weil@mail.berlios.de> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: QEMU Developers Am 01.07.2010 22:10, schrieb Blue Swirl: > On Thu, Jul 1, 2010 at 11:08 AM, Stefan Weil wrote: >> Signed-off-by: Stefan Weil >> --- >> 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