Re: [PATCH v8 09/19] scmi_protocols: update struct scmi_base_discover_list_protocols_out

[Marek Vasut <marex@denx.de>]

On 3/21/25 8:15 AM, Alice Guo (OSS) wrote:

[...]

> @@ -276,22 +276,31 @@ static int scmi_base_discover_list_protocols_int(struct udevice *dev,
>   	if (ret)
>   		return ret;
>   

This math here could really use a code comment to explain what is going 
on here.

I assume there is 1 byte per protocol in the struct 
scmi_base_discover_list_protocols_out { .protocols[] } member, right ? 
If so, why not simply change u32 protocols[] to u8 protocols[] and use some:

out_size = sizeof(*out) + roundup(num_protocols, sizeof(u32))

?

That might make the whole parsing using for (i = 0; i < 
out->num_protocols; i++, cur++) simpler too ?

> +	out_size = sizeof(*out) + sizeof(u32) * (1 + (num_protocols - 1) / 4);
> +	out = calloc(1, out_size);
> +	if (!out)
> +		return -ENOMEM;
> +	msg.out_msg = (u8 *)out;
> +	msg.out_msg_sz = out_size;
> +
>   	buf = calloc(sizeof(u8), num_protocols);
> -	if (!buf)
> +	if (!buf) {
> +		free(out);
>   		return -ENOMEM;
> +	}
>   
>   	cur = 0;
>   	do {
>   		ret = devm_scmi_process_msg(dev, &msg);
>   		if (ret)
>   			goto err;
> -		if (out.status) {
> -			ret = scmi_to_linux_errno(out.status);
> +		if (out->status) {
> +			ret = scmi_to_linux_errno(out->status);
>   			goto err;
>   		}
>   
> -		for (i = 0; i < out.num_protocols; i++, cur++)
> -			buf[cur] = out.protocols[i / 4] >> ((i % 4) * 8);
> +		for (i = 0; i < out->num_protocols; i++, cur++)
> +			buf[cur] = out->protocols[i / 4] >> ((i % 4) * 8);
>   	} while (cur < num_protocols);
>   
>   	*protocols = buf;
> @@ -299,6 +308,7 @@ static int scmi_base_discover_list_protocols_int(struct udevice *dev,
>   	return num_protocols;
>   err:
>   	free(buf);
> +	free(out);
>   
>   	return ret;
>   }
> diff --git a/include/scmi_protocols.h b/include/scmi_protocols.h
> index 519b906b4ce..9046de7e3e7 100644
> --- a/include/scmi_protocols.h
> +++ b/include/scmi_protocols.h
> @@ -145,7 +145,7 @@ struct scmi_base_discover_impl_version_out {
>   struct scmi_base_discover_list_protocols_out {
>   	s32 status;
>   	u32 num_protocols;
> -	u32 protocols[3];
> +	u32 protocols[];
>   };
[...]