Re: [PATCH] migration: Fix parse_ramblock() on overwritten retvals

[Peter Xu <peterx@redhat.com>]

On Thu, Oct 19, 2023 at 01:40:29PM +0100, Peter Maydell wrote:
> On Tue, 17 Oct 2023 at 21:40, Peter Xu <peterx@redhat.com> wrote:
> >
> > It's possible that some errors can be overwritten with success retval later
> > on, and then ignored.  Always capture all errors and report.
> >
> > Reported by Coverity 1522861, but actually I spot one more in the same
> > function.
> 
> The other one is CID 1522862, I think.

Yes..

> 
> > Fixes: CID 1522861
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> 
> > ---
> >  migration/ram.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/migration/ram.c b/migration/ram.c
> > index c844151ee9..d8bdb53a8f 100644
> > --- a/migration/ram.c
> > +++ b/migration/ram.c
> > @@ -3888,6 +3888,8 @@ static int parse_ramblock(QEMUFile *f, RAMBlock *block, ram_addr_t length)
> >          ret = qemu_ram_resize(block, length, &local_err);
> >          if (local_err) {
> >              error_report_err(local_err);
> > +            assert(ret < 0);
> 
> We usually don't bother asserting for this kind of "function
> reports errors two ways" code.

Juan, please feel free to drop the assert() if it's in the queue.

After this one lands, I'll send a patch to remove qemu_ram_resize retval
and only rely on Error*.

Thanks,

-- 
Peter Xu