[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] memory: Set notdirty_mem_ops validator
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH] memory: Set notdirty_mem_ops validator |
Date: |
Fri, 6 Sep 2019 08:44:08 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 |
On 9/6/19 8:24 AM, Philippe Mathieu-Daudé wrote:
>>>> static const MemoryRegionOps notdirty_mem_ops = {
>>>> .write = notdirty_mem_write,
>>>> - .valid.accepts = notdirty_mem_accepts,
>>>> .endianness = DEVICE_NATIVE_ENDIAN,
>>>> .valid = {
>>>> .min_access_size = 1,
>>>> .max_access_size = 8,
>>>> .unaligned = false,
>>>> + .accepts = notdirty_mem_accepts,
>>>
>>> I'm surprised the compiler doesn't emit any warning...
>>
>> Same here.
>>
>> But reading
>> https://en.cppreference.com/w/c/language/struct_initialization, this is
>> compliant behavior:
>>
>> "However, when an initializer begins with a left open brace, its current
>> object is fully re-initialized and any prior explicit initializers for
>> any of its subobjects are ignored:"
>>
>> so it is worth filing a gcc bug asking for a QoI improvement in adding a
>> warning (since the code does not violate the C standard, but does cause
>> surprises in the reinitialization of omitted members in the later {} to
>> go back to 0 in spite of the earlier initialization by nested name).
>
> Just remembered another case of (correct) reinitialization in
> hw/arm/palm.c:101:
We use nested reinitialization in multiple places. A gcc warning would
have to be discriminating enough to NOT warn merely when something is
listed twice...:
>
> static struct {
> int row;
> int column;
> } palmte_keymap[0x80] = {
> [0 ... 0x7f] = { -1, -1 },
> [0x3b] = { 0, 0 }, /* F1 -> Calendar */
Here, [0x3b].row and [0x3b].column are listed twice, but both of the
second listings are explicit.
Similarly, in qobject/json-lexer.c:
static const uint8_t json_lexer[][256] = {
/* Relies on default initialization to IN_ERROR! */
...
/*
* Two start states:
* - IN_START recognizes JSON tokens with our string extensions
* - IN_START_INTERP additionally recognizes interpolation.
*/
[IN_START ... IN_START_INTERP] = {
['"'] = IN_DQ_STRING,
...
['\n'] = IN_START,
},
[IN_START_INTERP]['%'] = IN_INTERP,
};
Done that way on purpose (I actually remember scratching my head on the
best way to compress things while reviewing Markus' patch that ended up
as 2cbd15aa6f; it took me a couple of tries off-list to end up at that
override).
...rather, the gcc warning that I envision should ONLY be when a later
partial ={} causes a zero-initialization override of an earlier explicit
.member, and NOT when a later complete ={} or explicit member overrides
an earlier init (whether the earlier one was explicit due to .member or
implicit due to partial ={}).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
- Re: [Qemu-devel] [PATCH] memory: Set notdirty_mem_ops validator, (continued)
Re: [Qemu-devel] [PATCH] memory: Set notdirty_mem_ops validator, Philippe Mathieu-Daudé, 2019/09/06