[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: Memory leak in transfer_memory_block()?
From: |
Zhanghailiang |
Subject: |
RE: Memory leak in transfer_memory_block()? |
Date: |
Thu, 18 Jun 2020 06:49:51 +0000 |
> -----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?