RE: Memory leak in transfer_memory_block()?

[Zhanghailiang <zhang.zhanghailiang@huawei.com>]

> -----Original Message-----
> From: Markus Armbruster [mailto:armbru@redhat.com]
> Sent: Thursday, June 18, 2020 1:36 PM
> To: Zhanghailiang <zhang.zhanghailiang@huawei.com>
> Cc: qemu-devel@nongnu.org; Michael Roth <mdroth@linux.vnet.ibm.com>
> Subject: Memory leak in transfer_memory_block()?
> 
> We appear to leak an Error object when ga_read_sysfs_file() fails with
> errno != ENOENT unless caller passes true @sys2memblk:
> 
>     static void transfer_memory_block(GuestMemoryBlock *mem_blk, bool
> sys2memblk,
>                                       GuestMemoryBlockResponse
> *result,
>                                       Error **errp)
>     {
>         [...]
>         if (local_err) {
> 
> We have an Error object.
> 
>             /* treat with sysfs file that not exist in old kernel */
>             if (errno == ENOENT) {
> 
> Case 1: ENOENT; we free it.  Good.
> 
>                 error_free(local_err);
>                 if (sys2memblk) {
>                     mem_blk->online = true;
>                     mem_blk->can_offline = false;
>                 } else if (!mem_blk->online) {
>                     result->response =
> 
> GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_NOT_SUPPORTED;
>                 }
>             } else {
> 
> Case 2: other than ENOENT
> 
>                 if (sys2memblk) {
> 
> Case 2a: sys2memblk; we pass it to the caller.  Good.
> 
>                     error_propagate(errp, local_err);
>                 } else {
> 
> Case 2b: !sys2memblk; ???
> 

Good catch!  I think we should pass the error info back to the caller,
Let's record this error for debug when it happens.

>                     result->response =
> 
> GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_FAILED;
>                 }
>             }
>             goto out2;
>         }
>         [...]
>     out2:
>         g_free(status);
>         close(dirfd);
>     out1:
>         if (!sys2memblk) {
>             result->has_error_code = true;
>             result->error_code = errno;
>         }
>     }
> 
> What is supposed to be done with @local_err in case 2b?