On Sat, 7 Feb 2026 at 10:24, Sergei Heifetz <[email protected]> wrote: > > On 2/4/26 17:53, Laurent Vivier wrote: > > Le 04/02/2026 à 08:57, Sergei Heifetz a écrit : > >> Reorder the code so the assertion of block occurs before it is > >> used in the subsequent lines. > >> > >> Signed-off-by: Sergei Heifetz <[email protected]> > >> --- > >> system/physmem.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/system/physmem.c b/system/physmem.c > >> index b0311f4531..4308e02940 100644 > >> --- a/system/physmem.c > >> +++ b/system/physmem.c > >> @@ -2054,11 +2054,11 @@ static int memory_try_enable_merging(void > >> *addr, size_t len) > >> */ > >> int qemu_ram_resize(RAMBlock *block, ram_addr_t newsize, Error **errp) > >> { > >> + assert(block); > >> + > >> const ram_addr_t oldsize = block->used_length; > >> const ram_addr_t unaligned_size = newsize; > >> - assert(block); > >> - > > > > According to coding style (docs/devel/style.rst): > > > > Mixed declarations (interleaving statements and declarations within > > blocks) are generally not allowed. > > > >> newsize = TARGET_PAGE_ALIGN(newsize); > >> newsize = REAL_HOST_PAGE_ALIGN(newsize); > > > > Thanks, > > Laurent > > Should I remove the assertion altogether, then? I think the const > qualifier on oldsize is more useful than the assertion.
I think personally I would drop the assertion. This falls into a category I think of as "assertion doesn't buy us anything". If block is NULL we're going to crash immediately when we dereference it, so the assert doesn't help to turn a hard-to-debug bug into an easy-to-debug bug (e.g. by catching incorrect values long before they're used) or to turn a nasty kind of failure into a safer one (e.g. by turning an array overrun into an assertion). And looking more closely, we call this function in only two places: * migration/ram.c, where we've already dereferenced block in the code just before the call * system/memory.c, which asserts mr->ram_block before passing it in So I think the assert in this function isn't buying us anything, and the simplest thing is to delete it. thanks -- PMM
