> -----Original Message----- > From: Markus Armbruster [mailto:arm...@redhat.com] > Sent: Thursday, June 18, 2020 1:36 PM > To: Zhanghailiang <zhang.zhanghaili...@huawei.com> > Cc: qemu-devel@nongnu.org; Michael Roth <mdr...@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?