[alsa-devel] [alsa-lib] A suggestion on file cards.c

[alex dot baldacchino dot alsasub at gmail dot com]

Hello,
first of all, let me point out documentation for snd_card_get_index is
incorrect: it is stated that zero is returned in case of success,
instead, a value greater than or equal to zero is the real success
result.

In second place, let me suggest a different implementation.

There's a number of functions (to get index, name, longname of a card;
other ones could be added eventually, as the interface could evolve)
resorting to a round of calls and pointer dereferencing to
snd_ctl_hw_open() and snd_ctl_card_info()/snd_ctl_hw_card_info() to
perform about the same operations as snd_card_load2() (eventually
combined with snd_card_load1() ), potentially in a more dangerous
fashion and possibly causing snd_card_load2() to be called anyway.
Thus, I'm wondering if such calls could be avoided with just a few
(not excessively invasive) changes to snd_card_load2() ( +
snd_card_load1(), if the case, as I think), and I've made an attempt
to implement those functions this way.

Let's consider a few things. Function snd_ctl_hw_open() is called with
the only purpose to allocate and populate an object of type snd_ctl_t,
whose pointer is then passed to snd_ctl_card_info(); the latter one
accesses that object fields to end up calling snd_ctl_hw_card_info(),
whose purpose is to call the same ioctl as snd_card_load2(), in order
to populate an object of type snd_ctl_card_info_t, therefore a call to
a (not too much) modified version of snd_card_load2 could be enough
for the same goals of successive calls to snd_ctl_hw_open() and
snd_ctl_card_info().

This would rise following constraints:

1) Creating an object of type snd_ctl_card_info_t accessible by the
caller after the function (snd_card_load2) returns. This is easy to
accomplish: the caller uses a local variable of that type, thus let's
just add an argument of type snd_ctl_card_info_t * and either use it
in place of the internal (automatic variable) one used by
snd_card_load2(), or choose whether to use the pointer argument (if
not null) or an automatic variable (I've chosen this path).

2) Accessing a device special file corresponding to an integer card
number. More reasoning needed here, since snd_ctl_hw_open() does it on
its own, whereas snd_card_load2 requires a char* argument. Translating
card number to device path can be done in three ways, essentially:

- building a proper path name the same way as snd_ctl_hw_open() does
it (that is, about the same way as snd_card_load1() does it - see
below) in each function calling snd_card_load2(); feasible, yet not
very good for maintenance;

- doing so inside snd_card_load2() if passed char pointer is null;
feasible, but it would still be duplicating (at least part of the)
code from snd_card_load1();

- calling snd_card_load2() via snd_card_load1() (as it's done right
now, but for one case - within snd_card_get_index - which would remain
about the same), by just furnishing snd_card_load1() with a
pointer-to-snd_ctl_card_info_t parameter to be passed straightforward
to snd_card_load2() (that could be null when specific card infos other
than card number aren't needed by the caller).

After a few considerations, the latter one seems feasible, and have
been my choice. Function snd_ctl_hw_open() builds the path name for a
control* device the same way it's done by snd_card_load1() (apparently
aload* devices aren't taken into account, but please, read further),
then snd_open_device is called, as it's done by snd_card_load2() BUT
with an important difference: the latter function opens devices
read-only, whereas snd_ctl_hw_open() can (and, as actually called by
snd_card_* stuff, it does) open devices read-write, which is both
needless and potentially dangerous. In case of failure opening chosen
device, snd_ctl_hw_open calls snd_card_load() and retries: this means
that snd_card_load1() and snd_card_load2() (via snd_card_load()) can
be called anyway, so that some operations are performed more than once
needlessly (and there's no need to call snd_card_load() from within
snd_card_get_index() anyway), but also that aload stuff, if compiled
as supported, is somehow taken into account silently even when it
would seem to be ignored (but perhaps shouldn't be?), as in
snd_card_get_name/longname. This should make snd_card_load1() suitable
to translate card number to device path and call snd_card_load2.

3) Printing out error messages. In some cases both snd_ctl_hw_open()
and snd_ctl_hw_card_info() can produce error messages, whereas
snd_card_load#() functions are mute in same scenarios. Whenever
they're called directly (in current implementation), such a behaviour
should remain as is, but when replacing snd_ctl_hw_* stuff same
errors/messages should be printed out. This is possible by duplicating
those (small) pieces of code from snd_ctl_hw_* functions and executing
them conditionally, availing of a further integer argument (used as a
boolean flag). To avoid complicating calls to snd_card_load1/2(), a
few macros can be defined to both mask out the "printing flag" and
give them an alias name appropriate for the context they're being
called from (which could add expressiveness to the implementation).

Anyway, take this as a simple suggestion, just give it a look.

===================================================
src/control/cards.c - attached
===================================================
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cards.c
Type: text/x-csrc
Size: 7266 bytes
Desc: not available
Url : http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20110605/d23cface/attachment.c