Alexandre Ratchov wrote:
> Initially aucat was designed to handle one device only so multiple
> audio devices used to be supported simply by starting multiple aucat
> processes (one per device), very simple. Then (~1.5 years ago) we
> added support for multiple audio devices (ie multiple -f options), and
> now it's time to finish the job and add a "device number" component in
> sndio(7) device names. Since this partially breaks compatibility, this
> is a opportunitiy to fix few other design mistakes (eg ':' being used
> by inet6, type name vs api name confusion, etc..). This leads to the
> following names:
>
> type[@hostname][,unit]/devnum[.option]
>
Hi Alexandre,
The highlighted code at the bottom of this mail contains comparisons like
this: strncmp("snd", str, len) == 0
This makes it possible to do things like:
$ aucat -fsnds/0 -i${HOME}/them.wav
snd0: snds/0: failed to open audio device => Ok, this should fail
$ aucat -fsn/0 -i${HOME}/them.wav => Not Ok ???, this plays the music
The second call doesn't fail because if len is smaller then strlen("snd")
than it potentially matches a partial "snd", allowing device types like "s"
and "sn" (in the case of type "snd"). I suppose this is not what you
intended.
I tend to do comparisons like this as follows,
strncmp(str, "snd", sizeof("snd")) == 0,
using the NUL character in "snd" as a sort of boundary check.
I'm not 100% sure though that this is always safe when strlen(str) <
sizeof("snd").
Maybe the safest option is to check if len == strlen("snd") before making
the comparison. This mail includes a PoC patch using this method.
After applying the patch aucat should behave like this:
$ aucat -fsnds/0 -i${HOME}/them.wav
snd0: snds/0: failed to open audio device
$ aucat -fsnd/0 -i${HOME}/them.wav => Ok, playing music
$ aucat -fsn/0 -i${HOME}/them.wav
snd0: sn/0: failed to open audio device
$ aucat -fs/0 -i${HOME}/them.wav
snd0: s/0: failed to open audio device
$ aucat -f/0 -i${HOME}/them.wav
snd0: /0: failed to open audio device
$ aucat -f0 -i${HOME}/them.wav
snd0: 0: failed to open audio device
Though I patched mio.c I didn't actually test midicat.
For this proof-of-concept I only tested aucat with device types snd and
rsnd, which seem to work for me.
Some other more or less random weirdness I found is that I can specify either
sun, sun: or sun:0 for the device and all three of them will play sound.
I'd expect only sun:0 to work. Device aucat hasn't got this problem. I haven't
tracked this down but I suspect some additional checking is going on outside
the sio_open function.
P.S.:
I added a comparison function sio_is_type. But I didn't know how to share it
properly between sio.c and mio.c so my solution might look a bit quick and
dirty. (especially the extern sio_is_type declaration in mio.c)
>
> Index: lib/libsndio/mio.c
> ===================================================================
> RCS file: /cvs/src/lib/libsndio/mio.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 mio.c
> --- lib/libsndio/mio.c 17 Oct 2011 21:09:11 -0000 1.12
> +++ lib/libsndio/mio.c 8 Nov 2011 19:18:19 -0000
> @@ -49,26 +46,23 @@ mio_open(const char *str, unsigned mode,
> if (str == NULL && !issetugid())
> str = getenv("MIDIDEVICE");
> if (str == NULL) {
> - hdl = mio_aucat_open("0", mode, nbio);
> + hdl = mio_aucat_open("/0", mode, nbio, 1);
> if (hdl != NULL)
> return hdl;
> - return mio_rmidi_open("0", mode, nbio);
> + return mio_rmidi_open("/0", mode, nbio);
> }
> - sep = strchr(str, ':');
> - if (sep == NULL) {
> - DPRINTF("mio_open: %s: ':' missing in device name\n", str);
> - return NULL;
> + for (len = 0; (c = str[len]) != '\0'; len++) {
> + /* XXX: remove ':' compat bits */
> + if (c == ':' || c == '@' || c == '/' || c == ',')
> + break;
> }
> - len = sep - str;
> - if (len == (sizeof(prefix_midithru) - 1) &&
> - memcmp(str, prefix_midithru, len) == 0)
> - return mio_aucat_open(sep + 1, mode, nbio);
> - if (len == (sizeof(prefix_aucat) - 1) &&
> - memcmp(str, prefix_aucat, len) == 0)
> - return mio_aucat_open(sep + 1, mode, nbio);
> - if (len == (sizeof(prefix_rmidi) - 1) &&
> - memcmp(str, prefix_rmidi, len) == 0)
> - return mio_rmidi_open(sep + 1, mode, nbio);
> + if (strncmp("snd", str, len) == 0 ||
> + strncmp("aucat", str, len) == 0)
> + return mio_aucat_open(str + len, mode, nbio, 0);
> + if (strncmp("midithru", str, len) == 0)
> + return mio_aucat_open(str + len, mode, nbio, 1);
> + if (strncmp("rmidi", str, len) == 0)
> + return mio_rmidi_open(str + len, mode, nbio);
> DPRINTF("mio_open: %s: unknown device type\n", str);
> return NULL;
> }
> Index: lib/libsndio/sio.c
> ===================================================================
> RCS file: /cvs/src/lib/libsndio/sio.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 sio.c
> --- lib/libsndio/sio.c 9 May 2011 17:34:14 -0000 1.6
> +++ lib/libsndio/sio.c 8 Nov 2011 19:18:20 -0000
> @@ -67,22 +55,22 @@ sio_open(const char *str, unsigned mode,
> if (str == NULL && !issetugid())
> str = getenv("AUDIODEVICE");
> if (str == NULL) {
> - for (b = backends; b->prefix != NULL; b++) {
> - hdl = b->open(NULL, mode, nbio);
> - if (hdl != NULL)
> - return hdl;
> - }
> - return NULL;
> + hdl = sio_aucat_open("/0", mode, nbio);
> + if (hdl != NULL)
> + return hdl;
> + return sio_sun_open("", mode, nbio);
> }
> - sep = strchr(str, ':');
> - if (sep == NULL) {
> - DPRINTF("sio_open: %s: ':' missing in device name\n", str);
> - return NULL;
> + for (len = 0; (c = str[len]) != '\0'; len++) {
> + /* XXX: remove ':' compat bits */
> + if (c == ':' || c == '@' || c == '/' || c == ',')
> + break;
> }
> - len = sep - str;
> - for (b = backends; b->prefix != NULL; b++) {
> - if (strlen(b->prefix) == len && memcmp(b->prefix, str, len) ==
> 0)
> - return b->open(sep + 1, mode, nbio);
> + if (strncmp("snd", str, len) == 0 ||
> + strncmp("aucat", str, len) == 0)
> + return sio_aucat_open(str + len, mode, nbio);
> + if (strncmp("rsnd", str, len) == 0 ||
> + strncmp("sun", str, len) == 0) {
> + return sio_sun_open(str + len, mode, nbio);
> }
> DPRINTF("sio_open: %s: unknown device type\n", str);
> return NULL;
--- sio.c Wed Nov 9 14:21:24 2011
+++ sio.c.new Wed Nov 9 14:09:44 2011
@@ -40,6 +40,17 @@
par->__magic = SIO_PAR_MAGIC;
}
+
+int
+sio_is_type(const char *str, size_t nstr, const char *type)
+{
+ if (nstr != strlen(type))
+ return (0);
+
+ return (strncmp(str, type, nstr) == 0);
+}
+
+
struct sio_hdl *
sio_open(const char *str, unsigned mode, int nbio)
{
@@ -65,13 +76,12 @@
if (c == ':' || c == '@' || c == '/' || c == ',')
break;
}
- if (strncmp("snd", str, len) == 0 ||
- strncmp("aucat", str, len) == 0)
+ if (sio_is_type(str, len, "snd") ||
+ sio_is_type(str, len, "aucat"))
return sio_aucat_open(str + len, mode, nbio);
- if (strncmp("rsnd", str, len) == 0 ||
- strncmp("sun", str, len) == 0) {
+ if (sio_is_type(str, len, "rsnd") ||
+ sio_is_type(str, len, "sun"))
return sio_sun_open(str + len, mode, nbio);
- }
DPRINTF("sio_open: %s: unknown device type\n", str);
return NULL;
}
--- mio.c Wed Nov 9 14:21:24 2011
+++ mio.c.new Wed Nov 9 14:24:25 2011
@@ -31,6 +31,8 @@
#include "debug.h"
#include "mio_priv.h"
+extern int sio_is_type(const char *, size_t, const char *);
+
struct mio_hdl *
mio_open(const char *str, unsigned mode, int nbio)
{
@@ -56,12 +58,12 @@
if (c == ':' || c == '@' || c == '/' || c == ',')
break;
}
- if (strncmp("snd", str, len) == 0 ||
- strncmp("aucat", str, len) == 0)
+ if (sio_is_type(str, len, "snd") ||
+ sio_is_type(str, len, "aucat"))
return mio_aucat_open(str + len, mode, nbio, 0);
- if (strncmp("midithru", str, len) == 0)
+ if (sio_is_type(str, len, "midithru"))
return mio_aucat_open(str + len, mode, nbio, 1);
- if (strncmp("rmidi", str, len) == 0)
+ if (sio_is_type(str, len, "rmidi"))
return mio_rmidi_open(str + len, mode, nbio);
DPRINTF("mio_open: %s: unknown device type\n", str);
return NULL;
