[PATCH v3 2/3] Cleanup execution of commands in dummy

[Boris Dušek]

From: Boris Dus?ek <address@hidden>
To: address@hidden

Current code that tries to play the dummy message using 3 command
line players is ugly and unmaintainable. Generalize it by storing
the 3 commands in an array, reading the array from config file and
iterating over the array to execute the commands.
---
 .gitignore                   |    1 +
 config/modules/Makefile.am   |    4 +--
 config/modules/dummy.conf.in |    1 +
 configure.ac                 |    5 ++++
 src/modules/dummy.c          |   63 +++++++++++++++++++++---------------------
 src/modules/module_utils.h   |   18 +++++++++++-
 6 files changed, 57 insertions(+), 35 deletions(-)
 create mode 100644 config/modules/dummy.conf.in

diff --git a/.gitignore b/.gitignore
index b479803..8d11b61 100644
--- a/.gitignore
+++ b/.gitignore
@@ -17,3 +17,4 @@ libtool
 py-compile
 speech-dispatcher.pc
 stamp-h1
+config/modules/dummy.conf
diff --git a/config/modules/Makefile.am b/config/modules/Makefile.am
index 8632db8..94f66ab 100644
--- a/config/modules/Makefile.am
+++ b/config/modules/Makefile.am
@@ -4,13 +4,13 @@ dist_moduleconf_DATA = cicero.conf espeak.conf festival.conf 
flite.conf \
                        ibmtts.conf ivona.conf dtk-generic.conf \
                        epos-generic.conf espeak-generic.conf \
                        espeak-mbrola-generic.conf llia_phon-generic.conf \
-                       swift-generic.conf
+                       swift-generic.conf dummy.conf
 
 dist_moduleconforig_DATA = cicero.conf espeak.conf festival.conf flite.conf \
                            ibmtts.conf ivona.conf dtk-generic.conf \
                            epos-generic.conf espeak-generic.conf \
                            espeak-mbrola-generic.conf llia_phon-generic.conf \
-                           swift-generic.conf
+                           swift-generic.conf dummy.conf
 
 if pico_support
 dist_moduleconf_DATA += pico.conf
diff --git a/config/modules/dummy.conf.in b/config/modules/dummy.conf.in
new file mode 100644
index 0000000..4bf08ed
--- /dev/null
+++ b/config/modules/dummy.conf.in
@@ -0,0 +1 @@
+PlayCommands @dummyplaycmds@
diff --git a/configure.ac b/configure.ac
index af45549..aa11669 100644
--- a/configure.ac
+++ b/configure.ac
@@ -358,11 +358,16 @@ AC_SUBST([audiodir])
 spdincludedir=${includedir}/speech-dispatcher
 AC_SUBST([spdincludedir])
 
+# Audio play commands for the dummy module
+dummyplaycmds="play aplay paplay"
+AC_SUBST([dummyplaycmds])
+
 AC_CONFIG_FILES([Makefile
                  speech-dispatcher.pc
                  config/Makefile
                  config/clients/Makefile
                  config/modules/Makefile
+                 config/modules/dummy.conf
                  doc/Makefile
                  include/Makefile
                  po/Makefile.in
diff --git a/src/modules/dummy.c b/src/modules/dummy.c
index 28fd5e4..8789a62 100644
--- a/src/modules/dummy.c
+++ b/src/modules/dummy.c
@@ -5,7 +5,7 @@
  * A simplific output module that just tries to play an
  * an error message in various ways.
  *
- * Copyright (C) 2008 Brailcom, o.p.s.
+ * Copyright (C) 2008, 2012 Brailcom, o.p.s.
  *
  * This is free software; you can redistribute it and/or modify it
  * under the terms of the GNU General Public License as published by
@@ -54,12 +54,15 @@ static void _dummy_child();
 
 /* Fill the module_info structure with pointers to this modules functions */
 
+MOD_OPTION_MORE(PlayCommands)
+
 /* Public functions */
 int module_load(void)
 {
-
        INIT_SETTINGS_TABLES();
 
+       MOD_OPTION_MORE_REG(PlayCommands);
+
        return 0;
 }
 
@@ -228,8 +231,23 @@ void _dummy_child()
 {
        sigset_t some_signals;
 
+       int i;
        int ret;
-       char *try1, *try2, *try3;
+
+       static const char *const default_play_cmds[] = {
+               "play",
+               "aplay",
+               "paplay"
+       };
+       const gchar *play_cmd = NULL;
+
+       if (!PlayCommands->len) {
+               // no commands specified for whatever reason - resort to the 
default ones
+               for (i = 0; i < 
sizeof(default_play_cmds)/sizeof(default_play_cmds[0]); ++i) {
+                       play_cmd = g_strdup(default_play_cmds[i]);
+                       g_array_append_val(PlayCommands, play_cmd);
+               }
+       }
 
        sigfillset(&some_signals);
        module_sigunblockusr(&some_signals);
@@ -237,43 +255,24 @@ void _dummy_child()
        DBG("Entering child loop\n");
        /* Read the waiting data */
 
-       try1 =
-           g_strdup("play " DATADIR
-                    "/dummy-message.wav > /dev/null 2> /dev/null");
-       try2 =
-           g_strdup("aplay  " DATADIR
-                    "/dummy-message.wav > /dev/null 2> /dev/null");
-       try3 =
-           g_strdup("paplay " DATADIR
-                    "/dummy-message.wav > /dev/null 2> /dev/null");
-
-       DBG("child: synth commands = |%s|%s|%s|", try1, try2, try3);
        DBG("Speaking in child...");
        module_sigblockusr(&some_signals);
 
-       ret = system(try1);
-       DBG("Executed shell command '%s' returned with %d", try1, ret);
-       if ((ret != 0)) {
-               DBG("Execution failed, trying seccond command");
-               ret = system(try2);
-               DBG("Executed shell command '%s' returned with %d", try1, ret);
-               if ((ret != 0)) {
-                       DBG("Execution failed, trying third command");
-                       ret = system(try3);
-                       DBG("Executed shell command '%s' returned with %d",
-                           try1, ret);
-                       if ((ret != 0) && (ret != 256)) {
-                               DBG("Failed, giving up.");
-                       }
+       for (i = 0; i < PlayCommands->len; ++i) {
+               play_cmd = g_array_index(PlayCommands, const gchar *, i);
+               gchar *const play_cmdline = g_strdup_printf("%s 
%s/dummy-message.wav > /dev/null 2> /dev/null", play_cmd, DATADIR);
+               DBG("Executing shell command '%s'", play_cmdline);
+               ret = system(play_cmdline);
+               g_free(play_cmdline);
+               if (ret != 0) {
+                       DBG("Executing command failed with return code %d", 
ret);
+               } else {
+                       break;
                }
        }
 
        module_sigunblockusr(&some_signals);
 
-       g_free(try1);
-       g_free(try2);
-       g_free(try3);
-
        DBG("Done, exiting from child.");
        exit(0);
 }
diff --git a/src/modules/module_utils.h b/src/modules/module_utils.h
index eacda04..0afb6be 100644
--- a/src/modules/module_utils.h
+++ b/src/modules/module_utils.h
@@ -1,7 +1,7 @@
 /*
  * module_utils.h - Module utilities
  *           Functions to help writing output modules for Speech Dispatcher
- * Copyright (C) 2003, 2004, 2007 Brailcom, o.p.s.
+ * Copyright (C) 2003, 2004, 2007, 2012 Brailcom, o.p.s.
  *
  * This is free software; you can redistribute it and/or modify it under the
  * terms of the GNU Lesser General Public License as published by the Free
@@ -269,6 +269,20 @@ configoption_t *add_config_option(configoption_t * options,
         return NULL; \
     }
 
+#define MOD_OPTION_MORE(name) \
+       GArray *name; \
+       \
+       DOTCONF_CB(name ## _cb) \
+       { \
+               int i; \
+               g_array_set_size(name, name->len + cmd->arg_count); \
+               for (i = 0; i < cmd->arg_count; ++i) { \
+                   const gchar *const val = g_strdup(cmd->data.list[i]); \
+                       g_array_index(name, const gchar *, name->len - 
cmd->arg_count + i) = val; \
+               } \
+               return NULL; \
+       }
+
 #define MOD_OPTION_2_HT(name, arg1, arg2) \
     typedef struct{ \
         char* arg1; \
@@ -342,6 +356,8 @@ configoption_t *add_config_option(configoption_t * options,
                                      ARG_INT, name ## _cb, NULL, 0);
 
 #define MOD_OPTION_MORE_REG(name) \
+       name = g_array_new(FALSE, FALSE, sizeof(gchar *)); \
+       g_array_set_clear_func(name, g_free); \
     module_dc_options = module_add_config_option(module_dc_options, \
                                      &module_num_dc_options, #name, \
                                      ARG_LIST, name ## _cb, NULL, 0);
-- 
1.7.9.6 (Apple Git-31.1)