On 09/12/18 16:28, Li Qiang wrote:
> Peter Maydell <peter.mayd...@linaro.org> 于2018年9月12日周三 下午8:55写道：
>
>> On 12 September 2018 at 13:32, Li Qiang <liq...@gmail.com> wrote:
>>> To avoid NULL-deref for the devices without read callbacks
>>>
>>> Signed-off-by: Li Qiang <liq...@gmail.com>
>>> ---
>>> memory.c | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/memory.c b/memory.c
>>> index 9b73892768..48d025426b 100644
>>> --- a/memory.c
>>> +++ b/memory.c
>>> @@ -406,6 +406,10 @@ static MemTxResult
>> memory_region_oldmmio_read_accessor(MemoryRegion *mr,
>>> {
>>> uint64_t tmp;
>>>
>>> + if (!mr->ops->old_mmio.read[ctz32(size)]) {
>>> + return MEMTX_DECODE_ERROR;
>>> + }
>>> +
>>> tmp = mr->ops->old_mmio.read[ctz32(size)](mr->opaque, addr);
>>> if (mr->subpage) {
>>> trace_memory_region_subpage_read(get_cpu_index(), mr, addr,
>> tmp, size);
>>> --
>>> 2.11.0
>>>
>>
>> There's patches on-list which drop the old_mmio field from the MemoryRegion
>> struct entirely, so I think this patch as it stands is obsolete.
>>
>> Currently our semantics are "you must provide both read and write, even
>> if one of them just always returns 0 / does nothing / returns an error".
>> We could probably reasonably assert this at the point when the
>> MemoryRegionOps is registered.
>>
>
> This patch is sent as the results of this thread:
> -->https://lists.gnu.org/archive/html/qemu-devel/2018-09/msg01332.html
>
> So I think I should send a path set to add all the missing read function
> as Laszlo Ersek points
> in the above thread discussion, right?
Can we introduce a central utility function at least (for a no-op read
returning 0), and initialize the read callbacks in question with the
address of that function?
Thanks,
Laszlo
