From patchwork Mon Apr 2 10:50:15 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= X-Patchwork-Id: 150114 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 4DF55B6EEB for ; Mon, 2 Apr 2012 21:42:22 +1000 (EST) Received: from localhost ([::1]:56734 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SEerB-0006RF-FA for incoming@patchwork.ozlabs.org; Mon, 02 Apr 2012 06:51:29 -0400 Received: from eggs.gnu.org ([208.118.235.92]:35261) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SEeqJ-0003sZ-VM for qemu-devel@nongnu.org; Mon, 02 Apr 2012 06:50:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SEeqH-0003k3-01 for qemu-devel@nongnu.org; Mon, 02 Apr 2012 06:50:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:43103) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SEeqG-0003jd-L6 for qemu-devel@nongnu.org; Mon, 02 Apr 2012 06:50:32 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q32AoVNk025730 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 2 Apr 2012 06:50:31 -0400 Received: from lettuce.camlab.fab.redhat.com (lettuce.camlab.fab.redhat.com [10.33.15.20]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q32AoNxE013508; Mon, 2 Apr 2012 06:50:30 -0400 From: "Daniel P. Berrange" To: qemu-devel@nongnu.org Date: Mon, 2 Apr 2012 11:50:15 +0100 Message-Id: <1333363816-1691-9-git-send-email-berrange@redhat.com> In-Reply-To: <1333363816-1691-1-git-send-email-berrange@redhat.com> References: <1333363816-1691-1-git-send-email-berrange@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH 8/9] Add more format string warning flags X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org From: "Daniel P. Berrange" Add -Wformat-contains-nul, -Wformat-extra-args, -Wformat-zero-length and -Wformat-nonliteral to the compiler flags & fix the issues they identify It is desirable to have these warnings enabled, even though it is not practical to fix all violations. Introduce some macros GCC_WARNINGS_SAVE, GCC_WARNINGS_RESTORE & GCC_WARNINGS_IGNORE, which allow specific warnings to be ignored for small blocks of code * cmd.c, block/vmdk.c: Pass format strings directly to snprintf instead of storing then in intermediate 'const char*' variables, so that GCC can validate args fully * configure: Add -Wformat-contains-nul, -Wformat-extra-args, -Wformat-zero-length, -Wformat-nonliteral * compiler.h: Add macros for turning off warnings selectively * bsd-user/strace.c, linux-user/strace.c: Disable warnings about non-literal format args * ui/vnc.c, ui/vnc.h, ui/vnc-auth-sasl.c: Refactor vnc_socket_local_addr & vnc_socket_remote_addr to just take a separator instead of format string Signed-off-by: Daniel P. Berrange --- block/vmdk.c | 64 ++++++++++++++++++++++++--------------------------- bsd-user/strace.c | 4 +++ cmd.c | 63 +++++++++++++++++++++++++++----------------------- compiler.h | 11 ++++++++ configure | 4 +++ linux-user/strace.c | 6 ++++ ui/vnc-auth-sasl.c | 7 ++++- ui/vnc.c | 20 ++++++++------- ui/vnc.h | 4 +- 9 files changed, 107 insertions(+), 76 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index 45c003a..12cad53 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1356,28 +1356,8 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options) char ext_desc_lines[BUF_SIZE] = ""; char path[PATH_MAX], prefix[PATH_MAX], postfix[PATH_MAX]; const int64_t split_size = 0x80000000; /* VMDK has constant split size */ - const char *desc_extent_line; char parent_desc_line[BUF_SIZE] = ""; uint32_t parent_cid = 0xffffffff; - const char desc_template[] = - "# Disk DescriptorFile\n" - "version=1\n" - "CID=%x\n" - "parentCID=%x\n" - "createType=\"%s\"\n" - "%s" - "\n" - "# Extent description\n" - "%s" - "\n" - "# The Disk Data Base\n" - "#DDB\n" - "\n" - "ddb.virtualHWVersion = \"%d\"\n" - "ddb.geometry.cylinders = \"%" PRId64 "\"\n" - "ddb.geometry.heads = \"16\"\n" - "ddb.geometry.sectors = \"63\"\n" - "ddb.adapterType = \"ide\"\n"; if (filename_decompose(filename, path, prefix, postfix, PATH_MAX)) { return -EINVAL; @@ -1411,11 +1391,6 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options) flat = !(strcmp(fmt, "monolithicFlat") && strcmp(fmt, "twoGbMaxExtentFlat")); compress = !strcmp(fmt, "streamOptimized"); - if (flat) { - desc_extent_line = "RW %lld FLAT \"%s\" 0\n"; - } else { - desc_extent_line = "RW %lld SPARSE \"%s\"\n"; - } if (flat && backing_file) { /* not supporting backing file for flat image */ return -ENOTSUP; @@ -1471,18 +1446,39 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options) /* Format description line */ snprintf(desc_line, sizeof(desc_line), - desc_extent_line, size / 512, desc_filename); + (flat + ? "RW %" PRId64 " FLAT \"%s\" 0\n" + : "RW %" PRId64 " SPARSE \"%s\"\n"), + size / 512, desc_filename); pstrcat(ext_desc_lines, sizeof(ext_desc_lines), desc_line); } /* generate descriptor file */ - snprintf(desc, sizeof(desc), desc_template, - (unsigned int)time(NULL), - parent_cid, - fmt, - parent_desc_line, - ext_desc_lines, - (flags & BLOCK_FLAG_COMPAT6 ? 6 : 4), - total_size / (int64_t)(63 * 16 * 512)); + snprintf(desc, sizeof(desc), + "# Disk DescriptorFile\n" + "version=1\n" + "CID=%x\n" + "parentCID=%x\n" + "createType=\"%s\"\n" + "%s" + "\n" + "# Extent description\n" + "%s" + "\n" + "# The Disk Data Base\n" + "#DDB\n" + "\n" + "ddb.virtualHWVersion = \"%d\"\n" + "ddb.geometry.cylinders = \"%" PRId64 "\"\n" + "ddb.geometry.heads = \"16\"\n" + "ddb.geometry.sectors = \"63\"\n" + "ddb.adapterType = \"ide\"\n", + (unsigned int)time(NULL), + parent_cid, + fmt, + parent_desc_line, + ext_desc_lines, + (flags & BLOCK_FLAG_COMPAT6 ? 6 : 4), + total_size / (int64_t)(63 * 16 * 512)); if (split || flat) { fd = open( filename, diff --git a/bsd-user/strace.c b/bsd-user/strace.c index d73bbca..73ae567 100644 --- a/bsd-user/strace.c +++ b/bsd-user/strace.c @@ -90,6 +90,9 @@ static const struct syscallname openbsd_scnames[] = { #include "openbsd/strace.list" }; + +GCC_WARNINGS_SAVE +GCC_WARNINGS_IGNORE("-Wformat-nonliteral") static void print_syscall(int num, const struct syscallname *scnames, unsigned int nscnames, abi_long arg1, abi_long arg2, abi_long arg3, @@ -119,6 +122,7 @@ print_syscall(int num, const struct syscallname *scnames, unsigned int nscnames, } gemu_log("Unknown syscall %d\n", num); } +GCC_WARNINGS_RESTORE static void print_syscall_ret(int num, abi_long ret, const struct syscallname *scnames, diff --git a/cmd.c b/cmd.c index 0806e18..0490d23 100644 --- a/cmd.c +++ b/cmd.c @@ -414,36 +414,41 @@ cvtnum( void cvtstr( - double value, - char *str, - size_t size) + double value, + char *str, + size_t size) { - const char *fmt; - int precise; - - precise = ((double)value * 1000 == (double)(int)value * 1000); - - if (value >= EXABYTES(1)) { - fmt = precise ? "%.f EiB" : "%.3f EiB"; - snprintf(str, size, fmt, TO_EXABYTES(value)); - } else if (value >= PETABYTES(1)) { - fmt = precise ? "%.f PiB" : "%.3f PiB"; - snprintf(str, size, fmt, TO_PETABYTES(value)); - } else if (value >= TERABYTES(1)) { - fmt = precise ? "%.f TiB" : "%.3f TiB"; - snprintf(str, size, fmt, TO_TERABYTES(value)); - } else if (value >= GIGABYTES(1)) { - fmt = precise ? "%.f GiB" : "%.3f GiB"; - snprintf(str, size, fmt, TO_GIGABYTES(value)); - } else if (value >= MEGABYTES(1)) { - fmt = precise ? "%.f MiB" : "%.3f MiB"; - snprintf(str, size, fmt, TO_MEGABYTES(value)); - } else if (value >= KILOBYTES(1)) { - fmt = precise ? "%.f KiB" : "%.3f KiB"; - snprintf(str, size, fmt, TO_KILOBYTES(value)); - } else { - snprintf(str, size, "%f bytes", value); - } + int precise; + + precise = ((double)value * 1000 == (double)(int)value * 1000); + + if (value >= EXABYTES(1)) { + snprintf(str, size, + precise ? "%.f EiB" : "%.3f EiB", + TO_EXABYTES(value)); + } else if (value >= PETABYTES(1)) { + snprintf(str, size, + precise ? "%.f PiB" : "%.3f PiB", + TO_PETABYTES(value)); + } else if (value >= TERABYTES(1)) { + snprintf(str, size, + precise ? "%.f TiB" : "%.3f TiB", + TO_TERABYTES(value)); + } else if (value >= GIGABYTES(1)) { + snprintf(str, size, + precise ? "%.f GiB" : "%.3f GiB", + TO_GIGABYTES(value)); + } else if (value >= MEGABYTES(1)) { + snprintf(str, size, + precise ? "%.f MiB" : "%.3f MiB", + TO_MEGABYTES(value)); + } else if (value >= KILOBYTES(1)) { + snprintf(str, size, + precise ? "%.f KiB" : "%.3f KiB", + TO_KILOBYTES(value)); + } else { + snprintf(str, size, "%f bytes", value); + } } struct timeval diff --git a/compiler.h b/compiler.h index 736e770..145d848 100644 --- a/compiler.h +++ b/compiler.h @@ -50,4 +50,15 @@ #define GCC_FMT_ATTR(n, m) #endif +#if defined __GNUC__ +# define GCC_WARNINGS_SAVE _Pragma("GCC diagnostic push") +# define GCC_WARNINGS_RESTORE _Pragma("GCC diagnostic pop") +# define DO_PRAGMA(x) _Pragma(#x) +# define GCC_WARNINGS_IGNORE(x) DO_PRAGMA(GCC diagnostic ignored x) +#else +# define GCC_WARNINGS_SAVE +# define GCC_WARNINGS_RESTORE +# define GCC_WARNINGS_IGNORE(x) +#endif + #endif /* COMPILER_H */ diff --git a/configure b/configure index 3d47440..175901f 100755 --- a/configure +++ b/configure @@ -1194,6 +1194,10 @@ gcc_flags="$gcc_flags -Wmissing-parameter-type" gcc_flags="$gcc_flags -Wuninitialized" gcc_flags="$gcc_flags -Wlogical-op" gcc_flags="$gcc_flags -Wmissing-format-attribute" +gcc_flags="$gcc_flags -Wformat-contains-nul" +gcc_flags="$gcc_flags -Wformat-extra-args" +gcc_flags="$gcc_flags -Wformat-zero-length" +gcc_flags="$gcc_flags -Wformat-nonliteral" cat > $TMPC << EOF int main(void) { return 0; } diff --git a/linux-user/strace.c b/linux-user/strace.c index 05a0d3e..75fd70b 100644 --- a/linux-user/strace.c +++ b/linux-user/strace.c @@ -627,6 +627,8 @@ print_string(abi_long addr, int last) * Prints out raw parameter using given format. Caller needs * to do byte swapping if needed. */ +GCC_WARNINGS_SAVE +GCC_WARNINGS_IGNORE("-Wformat-nonliteral") static void print_raw_param(const char *fmt, abi_long param, int last) { @@ -635,6 +637,7 @@ print_raw_param(const char *fmt, abi_long param, int last) (void) snprintf(format, sizeof (format), "%s%s", fmt, get_comma(last)); gemu_log(format, param); } +GCC_WARNINGS_RESTORE static void print_pointer(abi_long p, int last) @@ -1488,6 +1491,8 @@ static int nsyscalls = ARRAY_SIZE(scnames); /* * The public interface to this module. */ +GCC_WARNINGS_SAVE +GCC_WARNINGS_IGNORE("-Wformat-nonliteral") void print_syscall(int num, abi_long arg1, abi_long arg2, abi_long arg3, @@ -1513,6 +1518,7 @@ print_syscall(int num, } gemu_log("Unknown syscall %d\n", num); } +GCC_WARNINGS_RESTORE void diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c index 8fba770..07c4f5a 100644 --- a/ui/vnc-auth-sasl.c +++ b/ui/vnc-auth-sasl.c @@ -500,10 +500,13 @@ void start_auth_sasl(VncState *vs) VNC_DEBUG("Initialize SASL auth %d\n", vs->csock); /* Get local & remote client addresses in form IPADDR;PORT */ - if (!(localAddr = vnc_socket_local_addr("%s;%s", vs->csock))) + localAddr = vnc_socket_local_addr(';', vs->csock); + if (!localAddr) { goto authabort; + } - if (!(remoteAddr = vnc_socket_remote_addr("%s;%s", vs->csock))) { + remoteAddr = vnc_socket_remote_addr(';', vs->csock); + if (!remoteAddr) { g_free(localAddr); goto authabort; } diff --git a/ui/vnc.c b/ui/vnc.c index deb9ecd..86c5c3b 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -71,7 +71,7 @@ static void vnc_set_share_mode(VncState *vs, VncShareMode mode) } } -static char *addr_to_string(const char *format, +static char *addr_to_string(char separator, struct sockaddr_storage *sa, socklen_t salen) { char *addr; @@ -89,18 +89,19 @@ static char *addr_to_string(const char *format, return NULL; } - /* Enough for the existing format + the 2 vars we're + /* Enough for the separator + the 2 vars we're * substituting in. */ - addrlen = strlen(format) + strlen(host) + strlen(serv); + addrlen = strlen(host) + 1 + strlen(serv); addr = g_malloc(addrlen + 1); - snprintf(addr, addrlen, format, host, serv); + snprintf(addr, addrlen, "%s%c%s", host, separator, serv); addr[addrlen] = '\0'; return addr; } -char *vnc_socket_local_addr(const char *format, int fd) { +char *vnc_socket_local_addr(char separator, int fd) +{ struct sockaddr_storage sa; socklen_t salen; @@ -108,10 +109,11 @@ char *vnc_socket_local_addr(const char *format, int fd) { if (getsockname(fd, (struct sockaddr*)&sa, &salen) < 0) return NULL; - return addr_to_string(format, &sa, salen); + return addr_to_string(separator, &sa, salen); } -char *vnc_socket_remote_addr(const char *format, int fd) { +char *vnc_socket_remote_addr(char separator, int fd) +{ struct sockaddr_storage sa; socklen_t salen; @@ -119,7 +121,7 @@ char *vnc_socket_remote_addr(const char *format, int fd) { if (getpeername(fd, (struct sockaddr*)&sa, &salen) < 0) return NULL; - return addr_to_string(format, &sa, salen); + return addr_to_string(separator, &sa, salen); } static int put_addr_qdict(QDict *qdict, struct sockaddr_storage *sa, @@ -2857,7 +2859,7 @@ char *vnc_display_local_addr(DisplayState *ds) { VncDisplay *vs = ds ? (VncDisplay *)ds->opaque : vnc_display; - return vnc_socket_local_addr("%s:%s", vs->lsock); + return vnc_socket_local_addr(':', vs->lsock); } int vnc_display_open(DisplayState *ds, const char *display) diff --git a/ui/vnc.h b/ui/vnc.h index a851ebd..fc5be66 100644 --- a/ui/vnc.h +++ b/ui/vnc.h @@ -533,8 +533,8 @@ void buffer_append(Buffer *buffer, const void *data, size_t len); /* Misc helpers */ -char *vnc_socket_local_addr(const char *format, int fd); -char *vnc_socket_remote_addr(const char *format, int fd); +char *vnc_socket_local_addr(char separator, int fd); +char *vnc_socket_remote_addr(char separator, int fd); static inline uint32_t vnc_has_feature(VncState *vs, int feature) { return (vs->features & (1 << feature));