[alsa-devel] [PATCH] alsa-lib: issues in snd_seq_change_bit and some enhancements

[Takashi Iwai]

At Thu, 21 Feb 2008 06:12:19 -0300,
Aldrin Martoq wrote:
> 
> On Thu, Feb 21, 2008 at 4:59 AM, Clemens Ladisch <clemens at ladisch.de> wrote:
> > Aldrin Martoq wrote:
> >  > 2) There is no sane way to clear the event_filter of
> >  > snd_seq_client_info_t structure. Because the structure is opaque, we
> >  > cannot memset to zero because the user doesn't know the size of the
> >  > bitmap.
> >  The event type is an 8-bit quantity; this is part of the sequencer API.
> >  The bitmap has exactly 256 bits, i.e., 32 bytes.
> >  It would be nice if this were actually documented ...
> 
> Yup, I traced it to the sndrv_seq_client_info definition, but...
> 
> I think Takashi explicitly undocumented that stuff. The client_info
> and many other types/structures are hidden, the explanation is in the
> following link:
> typedef struct _snd_seq_client_info snd_seq_client_info_t;
> http://www.alsa-project.org/~tiwai/alsa.html#NewSeqAPI
> 
> So, my proposal follows the same spirit of hidding internal
> structures, which is kind of boring in C. But it's not boring if you
> are using some higher language (like python alsaseq, which I'm
> currently working on).

Yes, that looks nice to me.

> Another way to "resolve" this is to create a type instead of the raw
> pointer "unsigned char *". Something like:
> typedef struct snd_seq_client_info_event_filter {
>   unsigned char d[32];
> } snd_seq_client_info_event_filter_t;
> 
> and use the snd_seq_*_bit functions (btw, I added snd_seq_unset_bit);
> but in my opinion, creating functions instead of structures is more in
> the spirit of encapsulating the API for future extensions.

It's better to add new APIs rather than re-defining the type.

Could you split the patches?  I applied the fix for
snd_seq_change_bit() to HG tree now, as it's an obvious bug.
The others are rather extensions.

- a patch to add snd_seq_unset_bit()
- a patch to add new *_event_filter_*() APIs
- a patch to add new test code

Don't forget to give a proper changelog for each.


Thanks,

Takashi