| Message ID | 20230529075956.2235870-1-lizhijian@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
| Series | hw/clx: Fix CFMW config memory leak | expand |
On 30/05/2023 23:14, Jonathan Cameron via wrote: > On Mon, 29 May 2023 15:59:56 +0800 > Li Zhijian <lizhijian@cn.fujitsu.com> wrote: > >> Only 'fw' pointer is marked as g_autofree, so we shoud free other >> resource manually in error path. >> > Good spot. > > Patch title typo > hw/cxl:> OMG, it was the 2nd time i have this typo. i hate my stupid finger. > It's a bit annoying we can't handle this with more autofree magic. > That would work for fw->targets but not for the string duplicates. > > One other comment inline, > > Thanks, > > Jonathan > > >> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> >> --- >> hw/cxl/cxl-host.c | 10 ++++++++-- >> 1 file changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c >> index 034c7805b3e..dd1a7c83f71 100644 >> --- a/hw/cxl/cxl-host.c >> +++ b/hw/cxl/cxl-host.c >> @@ -48,7 +48,7 @@ static void cxl_fixed_memory_window_config(CXLState *cxl_state, >> if (object->size % (256 * MiB)) { >> error_setg(errp, >> "Size of a CXL fixed memory window must be a multiple of 256MiB"); >> - return; >> + goto err_free; > > >> } >> fw->size = object->size; >> >> @@ -57,7 +57,7 @@ static void cxl_fixed_memory_window_config(CXLState *cxl_state, >> cxl_interleave_granularity_enc(object->interleave_granularity, >> errp); >> if (*errp) { >> - return; >> + goto err_free; >> } >> } else { >> /* Default to 256 byte interleave */ >> @@ -68,6 +68,12 @@ static void cxl_fixed_memory_window_config(CXLState *cxl_state, >> g_steal_pointer(&fw)); >> >> return; >> + >> +err_free: >> + for (i = 0; i < fw->num_targets && fw->targets[i]; i++) { > > No need to check fw->targets[i] I knew it's safe to free a NULL pointer. The initial idea was just to make the loop end as soon as possible :) Of course, I would also be happy to delete this. Thanks Zhijian > >> + g_free(fw->targets[i]); >> + } >> + g_free(fw->targets); >> } >> >> void cxl_fmws_link_targets(CXLState *cxl_state, Error **errp) > >
diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c index 034c7805b3e..dd1a7c83f71 100644 --- a/hw/cxl/cxl-host.c +++ b/hw/cxl/cxl-host.c @@ -48,7 +48,7 @@ static void cxl_fixed_memory_window_config(CXLState *cxl_state, if (object->size % (256 * MiB)) { error_setg(errp, "Size of a CXL fixed memory window must be a multiple of 256MiB"); - return; + goto err_free; } fw->size = object->size; @@ -57,7 +57,7 @@ static void cxl_fixed_memory_window_config(CXLState *cxl_state, cxl_interleave_granularity_enc(object->interleave_granularity, errp); if (*errp) { - return; + goto err_free; } } else { /* Default to 256 byte interleave */ @@ -68,6 +68,12 @@ static void cxl_fixed_memory_window_config(CXLState *cxl_state, g_steal_pointer(&fw)); return; + +err_free: + for (i = 0; i < fw->num_targets && fw->targets[i]; i++) { + g_free(fw->targets[i]); + } + g_free(fw->targets); } void cxl_fmws_link_targets(CXLState *cxl_state, Error **errp)
Only 'fw' pointer is marked as g_autofree, so we shoud free other resource manually in error path. Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> --- hw/cxl/cxl-host.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)