Re: [PATCH v4 0/9] memory: assert and define MemoryRegionOps callbacks

[Philippe Mathieu-Daudé]

On 8/17/20 7:02 AM, P J P wrote:
> +-- On Sun, 16 Aug 2020, Philippe Mathieu-Daudé wrote --+
> | On 8/11/20 1:41 PM, P J P wrote:
> | > From: Prasad J Pandit <pjp@fedoraproject.org>
> | > * This series asserts that MemoryRegionOps objects define read/write
> | >   callback methods. Thus avoids potential NULL pointer dereference.
> | >   ex. -> 
> https://git.qemu.org/?p=qemu.git;a=commit;h=bb15013ef34617eb1344f5276292cadd326c21b2
> | > 
> | > * Also adds various undefined MemoryRegionOps read/write functions
> | >   to avoid potential assert failure.
> | 
> | What about read_with_attrs()/write_with_attrs()? It seems they are part of 
> | the same problem.
> 
> * read/write_with_attrs function is called if read/write callback is not 
>   defined
> 
>   ../softmmu/memory.c
>     if (mr->ops->write) {
>                     ... memory_region_write_accessor, mr,
>     } else {
>                     ... memory_region_write_with_attrs_accessor,
> 
>   So, defining read/write methods may also address read/write_with_attrs 
>   issue?
> 
> * $ grep -Eri -A 5 -B 5 '(\.read_with_attrs|\.write_with_attrs)' . | fpaste
> 
>    -> https://paste.centos.org/view/386c9597
> 
>   It doesn't show an occurrence where one of the read/write_with_attrs is 
>   missing.
> 
> * Nevertheless, if we need to define read/write_with_attrs routines, because 
>   memory_region_init_io() would assert(3) for them
> 
>   could that be a subsequent patch series please?

Yes no problem, I was just wondering and wasn't sure.