[PATCH v3 0/4] Fix a few minor issues reported by Coverity

[PATCH v3 0/4] Fix a few minor issues reported by Coverity

[Sergei Heifetz]

Hi. This patch series fixes several minor issues reported by our local
Coverity installation.

Changes in v3:
- Nothing. I sent the second version of the series without the corresponding 'v2'
  suffix by mistake. I apologise.

Changes in v2:
- Remove assert(block) in system/physmem.c instead of placing it
  before the dereference.

Sergei Heifetz (4):
  migration/savevm.c: reorder usage and assertion of mis->from_src_file
  dump/dump.c: reorder usage and assertion of block
  system/physmem.c: remove useless assertion of block
  hw/usb/core.c: reorder usage and assertion of p->ep

 dump/dump.c        | 2 +-
 hw/usb/core.c      | 2 +-
 migration/savevm.c | 3 ++-
 system/physmem.c   | 2 --
 4 files changed, 4 insertions(+), 5 deletions(-)

-- 
2.34.1

[PATCH v3 1/4] migration/savevm.c: reorder usage and assertion of mis->from_src_file

[Sergei Heifetz]

Reorder the code so the assertion of mis->from_src_file occurs before
the call to migration_ioc_unregister_yank_from_file, which dereferences
it in qemu_file_get_ioc.

Fixes: 39675ffffb3394 ("migration: Move the yank unregister of channel_close out")
Signed-off-by: Sergei Heifetz <heifetz@yandex-team.com>
---
 migration/savevm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 3dc812a7bb..930a3391e3 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2885,13 +2885,14 @@ static bool postcopy_pause_incoming(MigrationIncomingState *mis)
 
     assert(migrate_postcopy_ram());
 
+    assert(mis->from_src_file);
+
     /*
      * Unregister yank with either from/to src would work, since ioc behind it
      * is the same
      */
     migration_ioc_unregister_yank_from_file(mis->from_src_file);
 
-    assert(mis->from_src_file);
     qemu_file_shutdown(mis->from_src_file);
     qemu_fclose(mis->from_src_file);
     mis->from_src_file = NULL;
-- 
2.34.1

[PATCH v3 2/4] dump/dump.c: reorder usage and assertion of block

[Sergei Heifetz]

Reorder the code so the assertion of block occurs before it is
used in the subsequent lines.

Signed-off-by: Sergei Heifetz <heifetz@yandex-team.com>
---
 dump/dump.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dump/dump.c b/dump/dump.c
index f7a99a7af2..80ed6c8d21 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -1288,6 +1288,7 @@ static bool get_next_page(GuestPhysBlock **blockptr, uint64_t *pfnptr,
     /* block == NULL means the start of the iteration */
     if (!block) {
         block = QTAILQ_FIRST(&s->guest_phys_blocks.head);
+        assert(block);
         *blockptr = block;
         addr = block->target_start;
         *pfnptr = dump_paddr_to_pfn(s, addr);
@@ -1295,7 +1296,6 @@ static bool get_next_page(GuestPhysBlock **blockptr, uint64_t *pfnptr,
         *pfnptr += 1;
         addr = dump_pfn_to_paddr(s, *pfnptr);
     }
-    assert(block != NULL);
 
     while (1) {
         if (addr >= block->target_start && addr < block->target_end) {
-- 
2.34.1

[PATCH v3 3/4] system/physmem.c: remove useless assertion of block

[Sergei Heifetz]

It is useless to assert that block is not NULL because
it is already dereferenced in the first line of the function.

We could split the declaration and initialization of oldsize,
but then we would need to remove the const qualifier.
This seems worse, as the assertion would be almost useless anyway.

Signed-off-by: Sergei Heifetz <heifetz@yandex-team.com>
---
 system/physmem.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/system/physmem.c b/system/physmem.c
index b0311f4531..317b359ebe 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -2057,8 +2057,6 @@ int qemu_ram_resize(RAMBlock *block, ram_addr_t newsize, Error **errp)
     const ram_addr_t oldsize = block->used_length;
     const ram_addr_t unaligned_size = newsize;
 
-    assert(block);
-
     newsize = TARGET_PAGE_ALIGN(newsize);
     newsize = REAL_HOST_PAGE_ALIGN(newsize);
 
-- 
2.34.1

[PATCH v3 4/4] hw/usb/core.c: reorder usage and assertion of p->ep

[Sergei Heifetz]

Reorder the code so the assertion of p->ep occurs before it is
used in the subsequent lines.

Signed-off-by: Sergei Heifetz <heifetz@yandex-team.com>
---
 hw/usb/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/core.c b/hw/usb/core.c
index b3f811c513..9572a870cc 100644
--- a/hw/usb/core.c
+++ b/hw/usb/core.c
@@ -423,10 +423,10 @@ void usb_handle_packet(USBDevice *dev, USBPacket *p)
         p->status = USB_RET_NODEV;
         return;
     }
+    assert(p->ep);
     assert(dev == p->ep->dev);
     assert(dev->state == USB_STATE_DEFAULT);
     usb_packet_check_state(p, USB_PACKET_SETUP);
-    assert(p->ep != NULL);
 
     /* Submitting a new packet clears halt */
     if (p->ep->halted) {
-- 
2.34.1

Re: [PATCH v3 0/4] Fix a few minor issues reported by Coverity

[Michael Tokarev]

On 08.02.2026 13:39, Sergei Heifetz wrote:
> Hi. This patch series fixes several minor issues reported by our local
> Coverity installation.
> 
> Changes in v3:
> - Nothing. I sent the second version of the series without the corresponding 'v2'
>    suffix by mistake. I apologise.
> 
> Changes in v2:
> - Remove assert(block) in system/physmem.c instead of placing it
>    before the dereference.
> 
> Sergei Heifetz (4):
>    migration/savevm.c: reorder usage and assertion of mis->from_src_file
>    dump/dump.c: reorder usage and assertion of block
>    system/physmem.c: remove useless assertion of block
>    hw/usb/core.c: reorder usage and assertion of p->ep

Hi!

I applied 3 patches from this series - all but the physmem.c one.
It's ugly either way, so I'm not picking it up.  Sigh.

Sorry it took so long.

Thanks,

/mjt

Re: [PATCH v3 3/4] system/physmem.c: remove useless assertion of block

[Peter Maydell]

On Sun, 8 Feb 2026 at 10:41, Sergei Heifetz <heifetz@yandex-team.com> wrote:
>
> It is useless to assert that block is not NULL because
> it is already dereferenced in the first line of the function.
>
> We could split the declaration and initialization of oldsize,
> but then we would need to remove the const qualifier.
> This seems worse, as the assertion would be almost useless anyway.
>
> Signed-off-by: Sergei Heifetz <heifetz@yandex-team.com>

The commit message fails to note the information found during
review of the previous version:
 * this function is called from only two places
 * those places already either assert or assume that the
   block argument is not NULL
So the assertion is not getting us anything.

With the commit message updated:

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH v3 0/4] Fix a few minor issues reported by Coverity

[Peter Maydell]

On Wed, 11 Mar 2026 at 10:48, Michael Tokarev <mjt@tls.msk.ru> wrote:
>
> On 08.02.2026 13:39, Sergei Heifetz wrote:
> > Hi. This patch series fixes several minor issues reported by our local
> > Coverity installation.
> >
> > Changes in v3:
> > - Nothing. I sent the second version of the series without the corresponding 'v2'
> >    suffix by mistake. I apologise.
> >
> > Changes in v2:
> > - Remove assert(block) in system/physmem.c instead of placing it
> >    before the dereference.
> >
> > Sergei Heifetz (4):
> >    migration/savevm.c: reorder usage and assertion of mis->from_src_file
> >    dump/dump.c: reorder usage and assertion of block
> >    system/physmem.c: remove useless assertion of block
> >    hw/usb/core.c: reorder usage and assertion of p->ep
>
> Hi!
>
> I applied 3 patches from this series - all but the physmem.c one.
> It's ugly either way, so I'm not picking it up.  Sigh.

I think patch 3 is good, the commit message just didn't include
enough information to fully explain why the assert() there is pointless
(i.e. that it's called from only two places, which both already
assert or assume that block isn't NULL).

thanks
-- PMM

Re: [PATCH v3 3/4] system/physmem.c: remove useless assertion of block

[Sergei Heifetz]

On Wed Mar 11, 2026 at 3:51 PM +05, Peter Maydell wrote:
> On Sun, 8 Feb 2026 at 10:41, Sergei Heifetz <heifetz@yandex-team.com> wrote:
>>
>> It is useless to assert that block is not NULL because
>> it is already dereferenced in the first line of the function.
>>
>> We could split the declaration and initialization of oldsize,
>> but then we would need to remove the const qualifier.
>> This seems worse, as the assertion would be almost useless anyway.
>>
>> Signed-off-by: Sergei Heifetz <heifetz@yandex-team.com>
>
> The commit message fails to note the information found during
> review of the previous version:
>  * this function is called from only two places
>  * those places already either assert or assume that the
>    block argument is not NULL
> So the assertion is not getting us anything.
>
> With the commit message updated:
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> thanks
> -- PMM

Sorry, I should've included this information in the commit
message in the first place. I'll update it and resubmit
the series. Thank you.

Re: [PATCH v3 0/4] Fix a few minor issues reported by Coverity

[Sergei Heifetz]

On Wed Mar 11, 2026 at 3:47 PM +05, Michael Tokarev wrote:
> On 08.02.2026 13:39, Sergei Heifetz wrote:
>> Hi. This patch series fixes several minor issues reported by our local
>> Coverity installation.
>> 
>> Changes in v3:
>> - Nothing. I sent the second version of the series without the corresponding 'v2'
>>    suffix by mistake. I apologise.
>> 
>> Changes in v2:
>> - Remove assert(block) in system/physmem.c instead of placing it
>>    before the dereference.
>> 
>> Sergei Heifetz (4):
>>    migration/savevm.c: reorder usage and assertion of mis->from_src_file
>>    dump/dump.c: reorder usage and assertion of block
>>    system/physmem.c: remove useless assertion of block
>>    hw/usb/core.c: reorder usage and assertion of p->ep
>
> Hi!
>
> I applied 3 patches from this series - all but the physmem.c one.
> It's ugly either way, so I'm not picking it up.  Sigh.

I've just resubmitted the series with a change to the commit message of
the `physmem.c` patch. As pointed out by Peter Maydell, there is a good
reason to consider the assertion redundant.

> Sorry it took so long.

It's fine. Thank you.

Re: [PATCH v3 0/4] Fix a few minor issues reported by Coverity

[Michael Tokarev]

On 11.03.2026 16:21, Sergei Heifetz wrote:

> I've just resubmitted the series with a change to the commit message of
> the `physmem.c` patch. As pointed out by Peter Maydell, there is a good
> reason to consider the assertion redundant.

Sigh.. I already picked all the patches up.

Lemme re-pick stuff..

/mjt

Re: [PATCH v3 0/4] Fix a few minor issues reported by Coverity

[Sergei Heifetz]

On Wed Mar 11, 2026 at 6:25 PM +05, Michael Tokarev wrote:
> On 11.03.2026 16:21, Sergei Heifetz wrote:
>
>> I've just resubmitted the series with a change to the commit message of
>> the `physmem.c` patch. As pointed out by Peter Maydell, there is a good
>> reason to consider the assertion redundant.
>
> Sigh.. I already picked all the patches up.
>
> Lemme re-pick stuff..
>
> /mjt

Yeah, sorry about that. Thanks for your time.