On 6/18/21 3:04 PM, Peter Krempa wrote:
Count the elements in advance rather than using VIR_APPEND_ELEMENT and ensure that there's a NULL terminator for the string list so it's GStrv compatible.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/storage/storage_backend_iscsi_direct.c | 29 ++++++++-------------- 1 file changed, 11 insertions(+), 18 deletions(-)
diff --git a/src/storage/storage_backend_iscsi_direct.c b/src/storage/storage_backend_iscsi_direct.c index 263db835ae..2073a6df38 100644 --- a/src/storage/storage_backend_iscsi_direct.c +++ b/src/storage/storage_backend_iscsi_direct.c @@ -420,37 +420,30 @@ virISCSIDirectUpdateTargets(struct iscsi_context *iscsi, size_t *ntargets, char ***targets) { - int ret = -1; struct iscsi_discovery_address *addr; struct iscsi_discovery_address *tmp_addr; - size_t tmp_ntargets = 0; - char **tmp_targets = NULL; + + *ntargets = 0;
if (!(addr = iscsi_discovery_sync(iscsi))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to discover session: %s"), iscsi_get_error(iscsi)); - return ret; + return -1; }
- for (tmp_addr = addr; tmp_addr; tmp_addr = tmp_addr->next) { - g_autofree char *target = NULL; - - target = g_strdup(tmp_addr->target_name); + for (tmp_addr = addr; tmp_addr; tmp_addr = tmp_addr->next) + (*ntargets)++;
- if (VIR_APPEND_ELEMENT(tmp_targets, tmp_ntargets, target) < 0) - goto cleanup; - } + *targets = g_new0(char *, ntargets + 1);
I'm afraid ntargets + 1 will be too big on many systems. Please use: *ntargets + 1
+ *ntargets = 0;
- *targets = g_steal_pointer(&tmp_targets); - *ntargets = tmp_ntargets; - tmp_ntargets = 0; + for (tmp_addr = addr; tmp_addr; tmp_addr = tmp_addr->next) + *targets[(*ntargets)++] = g_strdup(tmp_addr->target_name);
Re-using *ntargets makes this a bit harder to follow, consider using 'i' and only filling *ntargets once. Jano
- ret = 0; - cleanup: iscsi_free_discovery_data(iscsi, addr); - virStringListFreeCount(tmp_targets, tmp_ntargets); - return ret; + + return 0; }
static int