From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50070) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WQMRb-0001YW-Gb for qemu-devel@nongnu.org; Wed, 19 Mar 2014 15:46:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WQMRP-0004ke-Hz for qemu-devel@nongnu.org; Wed, 19 Mar 2014 15:46:31 -0400 Sender: Paolo Bonzini Message-ID: <5329F406.7040606@redhat.com> Date: Wed, 19 Mar 2014 20:46:14 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1395247965-13889-1-git-send-email-pbonzini@redhat.com> <5329D4B7.4090602@redhat.com> In-Reply-To: <5329D4B7.4090602@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] scripts: add sample model file for Coverity Scan List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu-devel@nongnu.org Cc: qemu-trivial@nongnu.org, armbru@redhat.com Il 19/03/2014 18:32, Eric Blake ha scritto: >> + * >> + * Copyright (C) 2014 Red Hat, Inc. >> + * >> + * Authors: >> + * Markus Armbruster >> + * Paolo Bonzini >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2 or, at your >> + * option, any later version. See the COPYING file in the top-level directory. > > Aren't the license and authors blurbs usually in the other order? Not in the sample I copied from (migration.c). > >> +#define assert(x) if (!(x)) __coverity_panic__(); > > Will this break any 'if () assert(); else {}' blocks? Obviously, such > blocks already violate coding convention, but you might as well make > this definition safe to use for older code. Ok. >> + >> +static void __write(uint8_t *buf, int len) > > Will the fact that you used 'int len' instead of 'size_t' bite us on 32- > vs. 64-bit? Same for __read. Yeah, I copied this from address_space_rw. I'll change to ssize_t to catch negative values. > >> +void * >> +g_malloc0 (size_t n_bytes) >> +{ >> + void *mem; >> + __coverity_negative_sink__((ssize_t) n_bytes); >> + mem = calloc(1, n_bytes == 0 ? 1 : n_bytes); >> + if (!mem) __coverity_panic__ (); > > Is it worth being consistent on spacing before (? Yes. >> +void g_free (void *mem) >> +{ >> + if (mem) { >> + free(mem); >> + } > > Doesn't coverity already know that free(NULL) is a no-op, without you > having to repeat it? This part came from Markus. :) Paolo