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@xxxxxxxxxx>
> ---
> 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
>
