Re: [PATCH 3/4] system/physmem.c: reorder usage and assertion of block

BALATON Zoltan <[email protected]>
Newsgroups org.nongnu.qemu-trivial,org.nongnu.qemu-devel
Message-ID <[email protected]>
On Sat, 7 Feb 2026, Sergei Heifetz 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.

What does the const qualifier have to do with dereferencing block when 
it's NULL? You'd just need to split the line as:

int qemu_ram_resize(RAMBlock *block, ram_addr_t newsize, Error **errp)
{
    const ram_addr_t oldsize;
    const ram_addr_t unaligned_size = newsize;

    assert(block);
    oldsize = block->used_length;

How is that not obvious? The commit message also does not say why do you 
want to change this. How did you come up with this patch, what issue does 
it fix when block is NULL here?

Regards,
BALATON Zoltan
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.