Paolo Bonzini <pbonz...@redhat.com> writes:
> Il 19/03/2014 18:32, Eric Blake ha scritto:
>>> + *
>>> + * Copyright (C) 2014 Red Hat, Inc.
>>> + *
>>> + * Authors:
>>> + * Markus Armbruster <arm...@redhat.com>
>>> + * Paolo Bonzini <pbonz...@redhat.com>
>>> + *
>>> + * 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.
Coverity's builtin model models assert already, see
library/generic/libc/all/all.c. We shouldn't override it without a good
reason.
>>> +
>>> +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.
Change the real address_space_rw(), or the model's __write()?
>>> +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. :)
I don't know what possessed me when I wrote (or copied?) the silly
conditional. Let's drop it before someone else copies it.
