[alsa-devel] [alsa-lib] A suggestion on file cards.c
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 ===================================================
participants (1)
-
alex dot baldacchino dot alsasub at gmail dot com