Re: [PATCH] ALSA: control: Annotate snd_kcontrol with __counted_by()

["Gustavo A. R. Silva" <gustavo@xxxxxxxxxxxxxx>]

On 26/07/24 09:28, Takashi Iwai wrote:
> struct snd_kcontrol contains a flex array of snd_kcontrol_volatile
> objects at its end, and the array size is stored in count field.
> This can be annotated gracefully with __counted_by() for catching
> possible array overflows.
>
> One additional change is the order of the count field initialization;
> The assignment of the count field is moved before assignment of vd[]
> elements for avoiding false-positive warnings from compilers.

Thanks for catching this!

> Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>

Reviewed-by: Gustavo A. R. Silva <gustavoars@xxxxxxxxxx>

-
Gustavo

> ---
>
> This one seems out of Gustavo's radar.  I have no compiler that can
> actually check this annotation, so it's a bit untested.
>
>   include/sound/control.h | 2 +-
>   sound/core/control.c    | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/sound/control.h b/include/sound/control.h
> index c1659036c4a7..13511c50825f 100644
> --- a/include/sound/control.h
> +++ b/include/sound/control.h
> @@ -81,7 +81,7 @@ struct snd_kcontrol {
>   	unsigned long private_value;
>   	void *private_data;
>   	void (*private_free)(struct snd_kcontrol *kcontrol);
> -	struct snd_kcontrol_volatile vd[];	/* volatile data */
> +	struct snd_kcontrol_volatile vd[] __counted_by(count);	/* volatile data */
>   };
>   
>   #define snd_kcontrol(n) list_entry(n, struct snd_kcontrol, list)
> diff --git a/sound/core/control.c b/sound/core/control.c
> index f64a555f404f..c3aad64ffbf5 100644
> --- a/sound/core/control.c
> +++ b/sound/core/control.c
> @@ -237,11 +237,11 @@ static int snd_ctl_new(struct snd_kcontrol **kctl, unsigned int count,
>   	if (!*kctl)
>   		return -ENOMEM;
>   
> +	(*kctl)->count = count;
>   	for (idx = 0; idx < count; idx++) {
>   		(*kctl)->vd[idx].access = access;
>   		(*kctl)->vd[idx].owner = file;
>   	}
> -	(*kctl)->count = count;
>   
>   	return 0;
>   }