[PACH 0/2] virStorageBackendRBDGetVolNames: Fix memory leak and clean up

Peter Krempa (2): virStorageBackendRBDGetVolNames: Fix memory leak in 'rbd_list2' version virStorageBackendRBDGetVolNames: Refactor cleanup in 'rbd_list' version src/storage/storage_backend_rbd.c | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-) -- 2.31.1

The 'rbd_image_spec_t' struct has two string members 'id' and 'name'. We only stole the 'name' members thus the 'id's as well as the whole list would be leaked on success. Restructure the code so that we copy out the image names and call rbd_image_spec_list_cleanup on success rather than on error. The error path is then handled by using g_autofree for 'images'. Since we no longer have a error path after allocating the returned string list we can completely remove it's cleanup. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/storage/storage_backend_rbd.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index ce3ab11dd6..371ebfaf1b 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -579,9 +579,8 @@ static char ** virStorageBackendRBDGetVolNames(virStorageBackendRBDState *ptr) { char **names = NULL; - size_t nnames = 0; int rc; - rbd_image_spec_t *images = NULL; + g_autofree rbd_image_spec_t *images = NULL; size_t nimages = 16; size_t i; @@ -593,23 +592,18 @@ virStorageBackendRBDGetVolNames(virStorageBackendRBDState *ptr) break; if (rc != -ERANGE) { virReportSystemError(errno, "%s", _("Unable to list RBD images")); - goto error; + return NULL; } } names = g_new0(char *, nimages + 1); - nnames = nimages; for (i = 0; i < nimages; i++) - names[i] = g_steal_pointer(&images[i].name); + names[i] = g_strdup(images[i].name); - return names; - - error: - virStringListFreeCount(names, nnames); rbd_image_spec_list_cleanup(images, nimages); - VIR_FREE(images); - return NULL; + + return names; } #else /* ! WITH_RBD_LIST2 */ -- 2.31.1

On 6/15/21 4:16 PM, Peter Krempa wrote:
The 'rbd_image_spec_t' struct has two string members 'id' and 'name'. We only stole the 'name' members thus the 'id's as well as the whole list would be leaked on success.
Restructure the code so that we copy out the image names and call rbd_image_spec_list_cleanup on success rather than on error.
The error path is then handled by using g_autofree for 'images'.
Since we no longer have a error path after allocating the returned string list we can completely remove it's cleanup.
*its
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/storage/storage_backend_rbd.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Use automatic memory freeing for the string list so that we can remove the cleanup section. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/storage/storage_backend_rbd.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 371ebfaf1b..a4e8115dc4 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -611,7 +611,7 @@ virStorageBackendRBDGetVolNames(virStorageBackendRBDState *ptr) static char ** virStorageBackendRBDGetVolNames(virStorageBackendRBDState *ptr) { - char **names = NULL; + g_auto(GStrv) names = NULL; size_t nnames = 0; int rc; size_t max_size = 1024; @@ -626,7 +626,7 @@ virStorageBackendRBDGetVolNames(virStorageBackendRBDState *ptr) break; if (rc != -ERANGE) { virReportSystemError(errno, "%s", _("Unable to list RBD images")); - goto error; + return NULL; } VIR_FREE(namebuf); } @@ -640,18 +640,14 @@ virStorageBackendRBDGetVolNames(virStorageBackendRBDState *ptr) namedup = g_strdup(name); if (VIR_APPEND_ELEMENT(names, nnames, namedup) < 0) - goto error; + return NULL; name += strlen(name) + 1; } VIR_EXPAND_N(names, nnames, 1); - return names; - - error: - virStringListFreeCount(names, nnames); - return NULL; + return g_steal_pointer(&names); } #endif /* ! WITH_RBD_LIST2 */ -- 2.31.1

On 6/15/21 4:16 PM, Peter Krempa wrote:
Use automatic memory freeing for the string list so that we can remove the cleanup section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/storage/storage_backend_rbd.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Jano Tomko
-
Peter Krempa