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

On 2/7/26 16:16, BALATON Zoltan wrote:
> 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;

This code won't compile because you are trying to assign a new value to 
a const variable. That's precisely why you would need to remove the 
const qualifier from oldsize in order to keep the assert.

> 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? 

The original code contains a useless assert that comes after 
dereferencing block. The patch fixes that.