Re: [PATCH v3 2/2] ALSA: control: add ioctl to retrieve full card components

[Takashi Iwai]

On Tue, 03 Mar 2026 20:23:11 +0100,
Jaroslav Kysela wrote:
> 
> On 3/3/26 16:47, Takashi Iwai wrote:
> > On Tue, 03 Mar 2026 15:58:00 +0100,
> > Maciej Strozek wrote:
> >> 
> >> The fixed-size components field in SNDRV_CTL_IOCTL_CARD_INFO can be too
> >> small on systems with many audio devices.
> >> 
> >> Keep the existing struct snd_ctl_card_info ABI intact and add a new ioctl
> >> to retrieve the full components string.
> >> 
> >> When the legacy components field is truncated, append '>' to indicate
> >> that the full string is available via the new ioctl.
> >> 
> >> Link: https://github.com/alsa-project/alsa-lib/pull/494
> >> Suggested-by: Jaroslav Kysela <perex@xxxxxxxx>
> >> Suggested-by: Takashi Iwai <tiwai@xxxxxxxx>
> >> Signed-off-by: Maciej Strozek <mstrozek@xxxxxxxxxxxxxxxxxxxxx>
> >> ---
> >> Changes for v3:
> >>   - change components field to a dynamic array resizable in 32 byte increments
> >>   - removed SNDRV_CTL_COMPONENTS_LEN define
> >>   - sanity check if 'components' requests more than 512 bytes
> >>   - added a commit to clean up trailing whitespaces
> >>   - alsa-utils link no longer needed
> >> Changes for v2:
> >>   - do not modify existing card->components field
> >>   - add a new ioctl and struct to keep the full components string
> >>   - handle the split/trim in snd_ctl_card_info()
> >> ---
> >>   include/sound/core.h        |  4 ++--
> >>   include/uapi/sound/asound.h | 14 ++++++++++++-
> >>   sound/core/control.c        | 35 ++++++++++++++++++++++++++++++++-
> >>   sound/core/control_compat.c |  2 ++
> >>   sound/core/init.c           | 39 +++++++++++++++++++++++++++++--------
> >>   5 files changed, 82 insertions(+), 12 deletions(-)
> >> 
> >> diff --git a/include/sound/core.h b/include/sound/core.h
> >> index 4093ec82a0a1..2b58f79b524d 100644
> >> --- a/include/sound/core.h
> >> +++ b/include/sound/core.h
> >> @@ -87,8 +87,8 @@ struct snd_card {
> >>   	char longname[80];		/* name of this soundcard */
> >>   	char irq_descr[32];		/* Interrupt description */
> >>   	char mixername[80];		/* mixer name */
> >> -	char components[128];		/* card components delimited with
> >> -								space */
> >> +	char *components_ptr;
> >> +	unsigned int components_ptr_alloc_size; // current memory allocation components_ptr.
> >>   	struct module *module;		/* top-level module */
> >> 
> >>   	void *private_data;		/* private data for soundcard */
> >> diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
> >> index d3ce75ba938a..422b0b07613d 100644
> >> --- a/include/uapi/sound/asound.h
> >> +++ b/include/uapi/sound/asound.h
> >> @@ -1058,7 +1058,7 @@ struct snd_timer_tread {
> >>    *                                                                          *
> >>    ****************************************************************************/
> >> 
> >> -#define SNDRV_CTL_VERSION		SNDRV_PROTOCOL_VERSION(2, 0, 9)
> >> +#define SNDRV_CTL_VERSION		SNDRV_PROTOCOL_VERSION(2, 0, 10)
> >> 
> >>   struct snd_ctl_card_info {
> >>   	int card;			/* card number */
> >> @@ -1072,6 +1072,17 @@ struct snd_ctl_card_info {
> >>   	unsigned char components[128];	/* card components / fine identification, delimited with one space (AC97 etc..) */
> >>   };
> >> 
> >> +/*
> >> + * Card components can exceed the fixed 128 bytes in snd_ctl_card_info.
> >> + * Use SNDRV_CTL_IOCTL_CARD_COMPONENTS to retrieve the full string.
> >> + *
> >> + */
> >> +struct snd_ctl_card_components {
> >> +	int card;
> >> +	unsigned int length;
> >> +	unsigned char *components;
> >> +};
> > 
> > Embedding a pointer for ioctl is a nightmare for 32bit compatibility,
> > and you seem to have forgotten it, too ;)
> 
> It's doable and we have other more important structures like struct
> snd_xferi using those pointers. But yes, the translation is missing in
> this patch set. I already gave hint that control_compat.c must be
> modified, too.
> 
> > IMO, in this case, it'd be easier to use a flex array instead, e.g.
> > 
> > struct snd_ctl_card_components {
> > 	int card;
> > 	unsigned int length;
> > 	unsigned char components[];
> > };
> 
> There is an issue that flex arrays will break _IOC_SIZE().

That's true.  OTOH, having a pointer for the external data to be
copied means that _IOC_SIZE() doesn't cover the whole size but misses
the data, too.  So it's a question which implementation is uglier (or
easier to handle), after all.

BTW, it might be better to have both length and actual_length fields
in srtuct snd_ctl_card_components: the former specifies the allocated
buffer length while the latter is stored by kernel pointing to the
actual data size.  We can overwrite length field in return, but it
might be confusing when releasing a resource.


thanks,

Takashi