hw/cxl/cxl-host.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
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(-)
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)
--
2.31.1
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: 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] > + g_free(fw->targets[i]); > + } > + g_free(fw->targets); > } > > void cxl_fmws_link_targets(CXLState *cxl_state, Error **errp)
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) > >
© 2016 - 2024 Red Hat, Inc.