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. thanks -- PMM