Re: [PATCH] ethdev: prefer offload names in logs

[Ferruh Yigit <ferruh.yigit@amd.com>]

On 3/9/2023 8:16 AM, David Marchand wrote:
> +static char *
> +eth_dev_offload_names(uint64_t bitmask, const char *(*offload_name)(uint64_t))
> +{
> +	uint64_t offload;
> +	char *names;
> +
> +	if (bitmask == 0) {
> +		names = strdup("");
> +		goto out;
> +	}
> +
> +	offload = RTE_BIT64(__builtin_ctzll(bitmask));
> +	names = strdup(offload_name(offload));
> +	if (names == NULL)
> +		goto out;
> +
> +	bitmask &= ~offload;
> +	while (bitmask != 0) {
> +		char *old = names;
> +		int ret;
> +
> +		offload = RTE_BIT64(__builtin_ctzll(bitmask));
> +		ret = asprintf(&names, "%s,%s", old, offload_name(offload));
> +		free(old);
> +		if (ret == -1) {
> +			names = NULL;
> +			goto out;
> +		}
> +
> +		bitmask &= ~offload;
> +	}
> +
> +out:
> +	return names;
> +}
> +

Above works fine, not a strong opinion but just a comment,
this function is just for logging and output string will be short lived,
but it does lots of memory alloc and free, and expose lot of failure points.

To simplify, why not just alloc a big enough buffer, fill it in a loop
and never return NULL?
(Can always append "%s," and drop final ',' at the end.)