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

[PATCH v4 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 v4:
- Included Peter's explanation why `assert` is redundant in the commit
  message of the third patch.
- Added `Reviewed-by` tags that I missed in v2 to the other three patches.

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 v4 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>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 migration/savevm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 3dc812a7bbb..930a3391e35 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

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

[Fabiano Rosas]

Sergei Heifetz <heifetz@yandex-team.com> writes:

> 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>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  migration/savevm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 3dc812a7bbb..930a3391e35 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;

Acked-by: Fabiano Rosas <farosas@suse.de>

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

[Prasad Pandit]

On Wed, 11 Mar 2026 at 18:47, Sergei Heifetz <heifetz@yandex-team.com> wrote:
> 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>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  migration/savevm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 3dc812a7bbb..930a3391e35 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;

*  Change looks okay. But is it really possible that we reach the
Postcopy pause step with mis->from/to_src_file = NULL?
    Maybe we could move the following 'assert(mis->to_src_file);' to
the top too? Make it fail early before any call(s) further.
===
diff --git a/migration/savevm.c b/migration/savevm.c
index 197c89e0e6..18264feaac 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2874,6 +2874,8 @@ static bool
postcopy_pause_incoming(MigrationIncomingState *mis)
     trace_postcopy_pause_incoming();

     assert(migrate_postcopy_ram());
+    assert(mis->from_src_file);
+    assert(mis->to_src_file);

     /*
      * Unregister yank with either from/to src would work, since ioc behind it
@@ -2881,12 +2883,10 @@ static bool
postcopy_pause_incoming(MigrationIncomingState *mis)
      */
     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;

-    assert(mis->to_src_file);
     qemu_file_shutdown(mis->to_src_file);
     qemu_mutex_lock(&mis->rp_mutex);
     qemu_fclose(mis->to_src_file);
===

Thank you.
---
  - Prasad

[PATCH v4 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>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 dump/dump.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dump/dump.c b/dump/dump.c
index f7a99a7af2e..80ed6c8d219 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 v4 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.

The assertion is also unnecessary because the function is called
in only two places, and `block` can't be NULL in either of them:
- In `migration/ram.c`, we have already dereferenced `block` in
  the code just before the call.
- In `system/memory.c`, we assert `mr->ram_block` before passing
  it to the function.

(We could split the declaration and initialization of oldsize,
but then we would need to remove the const qualifier. As the
assertion is useless anyway, removing the const qualifier seems
worse.)

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

diff --git a/system/physmem.c b/system/physmem.c
index b0311f45312..317b359ebe2 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 v4 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>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 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 b3f811c513b..9572a870cc0 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 v4 0/4] Fix a few minor issues reported by Coverity

[Peter Xu]

On Wed, Mar 11, 2026 at 06:15:18PM +0500, Sergei Heifetz wrote:
> Hi. This patch series fixes several minor issues reported by our local
> Coverity installation.
> 
> Changes in v4:
> - Included Peter's explanation why `assert` is redundant in the commit
>   message of the third patch.
> - Added `Reviewed-by` tags that I missed in v2 to the other three patches.
> 
> 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

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu