[asterisk-commits] rizzo: trunk r94191 - in /trunk: channels/ include/asterisk/
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Thu Dec 20 06:56:08 CST 2007
Author: rizzo
Date: Thu Dec 20 06:56:07 2007
New Revision: 94191
URL: http://svn.digium.com/view/asterisk?view=rev&rev=94191
Log:
add some macros to simplify parsing the config file,
see description in config.h .
They are a variant of the set of macros i used in chan_oss.c,
structured in a way to be more robust to the presence of
spurious ';' - basically, they define wrappers for 'do {'
and '} while (0)', plus some helper functions to deal with simple
cases such as ast_copy_string, ast_malloc, strtoul, ast_true ...
The prefix (CV_ as 'Config Variable') tries to be easy to remember
and has been chosen to not conflict with other existing macros in the tree.
For the time being, I have only updated the three source files in the
tree that used the old M_* macros. Hopefully, more files will be
converted.
NOTE:
I understand that inventing my own dialect of C is generally wrong;
however, the lack of adequate support in the language encourages
lazy programming practices (such as ignoring errors, bounds, etc.)
and this increases the chance of vulnerability in the code, especially
because we are parsing user input here.
Hopefully, these macros and the use of ast_parse_arg (in config.h)
should encourage the programmer to write more robust code.
Modified:
trunk/channels/chan_oss.c
trunk/channels/chan_usbradio.c
trunk/channels/console_video.c
trunk/include/asterisk/config.h
Modified: trunk/channels/chan_oss.c
URL: http://svn.digium.com/view/asterisk/trunk/channels/chan_oss.c?view=diff&rev=94191&r1=94190&r2=94191
==============================================================================
--- trunk/channels/chan_oss.c (original)
+++ trunk/channels/chan_oss.c Thu Dec 20 06:56:07 2007
@@ -176,34 +176,6 @@
.. and so on for the other cards.
*/
-
-/*
- * Helper macros to parse config arguments. They will go in a common
- * header file if their usage is globally accepted. In the meantime,
- * we define them here. Typical usage is as below.
- * Remember to open a block right before M_START (as it declares
- * some variables) and use the M_* macros WITHOUT A SEMICOLON:
- *
- * {
- * M_START(v->name, v->value)
- *
- * M_BOOL("dothis", x->flag1)
- * M_STR("name", x->somestring)
- * M_F("bar", some_c_code)
- * M_END(some_final_statement)
- * ... other code in the block
- * }
- *
- * XXX NOTE these macros should NOT be replicated in other parts of asterisk.
- * Likely we will come up with a better way of doing config file parsing.
- */
-#define M_START(var, val) \
- const char *__s = var; const char *__val = val;
-#define M_END(x) x;
-#define M_F(tag, f) if (!strcasecmp((__s), tag)) { f; } else
-#define M_BOOL(tag, dst) M_F(tag, (dst) = ast_true(__val) )
-#define M_UINT(tag, dst) M_F(tag, (dst) = strtoul(__val, NULL, 0) )
-#define M_STR(tag, dst) M_F(tag, ast_copy_string(dst, __val, sizeof(dst)))
/*
* The following parameters are used in the driver:
@@ -1559,7 +1531,7 @@
static void store_config_core(struct chan_oss_pvt *o, const char *var, const char *value)
{
- M_START(var, value);
+ CV_START(var, value);
/* handle jb conf */
if (!ast_jb_read_conf(&global_jbconf, (char *)var,(char *) value))
@@ -1567,22 +1539,22 @@
if (!console_video_config(&o->env, var, value))
return;
- M_BOOL("autoanswer", o->autoanswer)
- M_BOOL("autohangup", o->autohangup)
- M_BOOL("overridecontext", o->overridecontext)
- M_STR("device", o->device)
- M_UINT("frags", o->frags)
- M_UINT("debug", oss_debug)
- M_UINT("queuesize", o->queuesize)
- M_STR("context", o->ctx)
- M_STR("language", o->language)
- M_STR("mohinterpret", o->mohinterpret)
- M_STR("extension", o->ext)
- M_F("mixer", store_mixer(o, value))
- M_F("callerid", store_callerid(o, value))
- M_F("boost", store_boost(o, value))
-
- M_END(/* */);
+ CV_BOOL("autoanswer", o->autoanswer);
+ CV_BOOL("autohangup", o->autohangup);
+ CV_BOOL("overridecontext", o->overridecontext);
+ CV_STR("device", o->device);
+ CV_UINT("frags", o->frags);
+ CV_UINT("debug", oss_debug);
+ CV_UINT("queuesize", o->queuesize);
+ CV_STR("context", o->ctx);
+ CV_STR("language", o->language);
+ CV_STR("mohinterpret", o->mohinterpret);
+ CV_STR("extension", o->ext);
+ CV_F("mixer", store_mixer(o, value));
+ CV_F("callerid", store_callerid(o, value)) ;
+ CV_F("boost", store_boost(o, value));
+
+ CV_END;
}
/*!
Modified: trunk/channels/chan_usbradio.c
URL: http://svn.digium.com/view/asterisk/trunk/channels/chan_usbradio.c?view=diff&rev=94191&r1=94190&r2=94191
==============================================================================
--- trunk/channels/chan_usbradio.c (original)
+++ trunk/channels/chan_usbradio.c Thu Dec 20 06:56:07 2007
@@ -201,34 +201,6 @@
END_CONFIG
*/
-
-/*! \brief
- * Helper macros to parse config arguments. They will go in a common
- * header file if their usage is globally accepted. In the meantime,
- * we define them here. Typical usage is as below.
- * Remember to open a block right before M_START (as it declares
- * some variables) and use the M_* macros WITHOUT A SEMICOLON:
- *
- * {
- * M_START(v->name, v->value)
- *
- * M_BOOL("dothis", x->flag1)
- * M_STR("name", x->somestring)
- * M_F("bar", some_c_code)
- * M_END(some_final_statement)
- * ... other code in the block
- * }
- *
- * XXX NOTE these macros should NOT be replicated in other parts of asterisk.
- * Likely we will come up with a better way of doing config file parsing.
- */
-#define M_START(var, val) \
- const char *__s = var; const char *__val = val;
-#define M_END(x) x;
-#define M_F(tag, f) if (!strcasecmp((__s), tag)) { f; } else
-#define M_BOOL(tag, dst) M_F(tag, (dst) = ast_true(__val) )
-#define M_UINT(tag, dst) M_F(tag, (dst) = strtoul(__val, NULL, 0) )
-#define M_STR(tag, dst) M_F(tag, ast_copy_string(dst, __val, sizeof(dst)))
/*!
* The following parameters are used in the driver:
@@ -2414,45 +2386,35 @@
o->spkrmax = amixer_max(o->devicenum, MIXER_PARAM_SPKR_PLAYBACK_VOL);
/* fill other fields from configuration */
for (v = ast_variable_browse(cfg, ctg); v; v = v->next) {
- M_START(v->name, v->value);
/* handle jb conf */
if (!ast_jb_read_conf(&global_jbconf, v->name, v->value))
continue;
-
-#if 0
- M_BOOL("autoanswer", o->autoanswer)
- M_BOOL("autohangup", o->autohangup)
- M_BOOL("overridecontext", o->overridecontext)
- M_STR("context", o->ctx)
- M_STR("language", o->language)
- M_STR("mohinterpret", o->mohinterpret)
- M_STR("extension", o->ext)
- M_F("callerid", store_callerid(o, v->value))
-#endif
- M_UINT("frags", o->frags)
- M_UINT("queuesize", o->queuesize)
- M_UINT("devicenum", o->devicenum)
- M_UINT("debug", usbradio_debug)
- M_BOOL("rxcpusaver", o->rxcpusaver)
- M_BOOL("txcpusaver", o->txcpusaver)
- M_BOOL("invertptt", o->invertptt)
- M_F("rxdemod", store_rxdemod(o, v->value))
- M_BOOL("txprelim", o->txprelim);
- M_F("txmixa", store_txmixa(o, v->value))
- M_F("txmixb", store_txmixb(o, v->value))
- M_F("carrierfrom", store_rxcdtype(o, v->value))
- M_F("rxsdtype", store_rxsdtype(o, v->value))
- M_F("rxctcssfreq", store_rxctcssfreq(o, v->value))
- M_F("txctcssfreq", store_txctcssfreq(o, v->value))
- M_F("rxgain", store_rxgain(o, v->value))
- M_BOOL("rxboostset", o->rxboostset)
- M_UINT("rxctcssrelax", o->rxctcssrelax)
- M_F("txtoctype", store_txtoctype(o, v->value))
- M_UINT("hdwtype", o->hdwtype)
- M_UINT("duplex", o->radioduplex)
- M_END(;
- );
+ CV_START(v->name, v->value);
+
+ CV_UINT("frags", o->frags);
+ CV_UINT("queuesize", o->queuesize);
+ CV_UINT("devicenum", o->devicenum);
+ CV_UINT("debug", usbradio_debug);
+ CV_BOOL("rxcpusaver", o->rxcpusaver);
+ CV_BOOL("txcpusaver", o->txcpusaver);
+ CV_BOOL("invertptt", o->invertptt);
+ CV_F("rxdemod", store_rxdemod(o, v->value));
+ CV_BOOL("txprelim", o->txprelim);;
+ CV_F("txmixa", store_txmixa(o, v->value));
+ CV_F("txmixb", store_txmixb(o, v->value));
+ CV_F("carrierfrom", store_rxcdtype(o, v->value));
+ CV_F("rxsdtype", store_rxsdtype(o, v->value));
+ CV_F("rxctcssfreq", store_rxctcssfreq(o, v->value));
+ CV_F("txctcssfreq", store_txctcssfreq(o, v->value));
+ CV_F("rxgain", store_rxgain(o, v->value));
+ CV_BOOL("rxboostset", o->rxboostset);
+ CV_UINT("rxctcssrelax", o->rxctcssrelax);
+ CV_F("txtoctype", store_txtoctype(o, v->value));
+ CV_UINT("hdwtype", o->hdwtype);
+ CV_UINT("duplex", o->radioduplex);
+
+ CV_END;
}
cfg1 = ast_config_load(config1, config_flags);
@@ -2468,16 +2430,15 @@
} else {
for (v = ast_variable_browse(cfg1, ctg); v; v = v->next) {
- M_START(v->name, v->value);
- M_UINT("rxmixerset", o->rxmixerset)
- M_UINT("txmixaset", o->txmixaset)
- M_UINT("txmixbset", o->txmixbset)
- M_F("rxvoiceadj", store_rxvoiceadj(o, v->value))
- M_F("rxctcssadj", store_rxctcssadj(o, v->value))
- M_UINT("txctcssadj", o->txctcssadj);
- M_UINT("rxsquelchadj", o->rxsquelchadj)
- M_END(;
- );
+ CV_START(v->name, v->value);
+ CV_UINT("rxmixerset", o->rxmixerset);
+ CV_UINT("txmixaset", o->txmixaset);
+ CV_UINT("txmixbset", o->txmixbset);
+ CV_F("rxvoiceadj", store_rxvoiceadj(o, v->value));
+ CV_F("rxctcssadj", store_rxctcssadj(o, v->value));
+ CV_UINT("txctcssadj", o->txctcssadj);
+ CV_UINT("rxsquelchadj", o->rxsquelchadj);
+ CV_END;
}
ast_config_destroy(cfg1);
}
Modified: trunk/channels/console_video.c
URL: http://svn.digium.com/view/asterisk/trunk/channels/console_video.c?view=diff&rev=94191&r1=94190&r2=94191
==============================================================================
--- trunk/channels/console_video.c (original)
+++ trunk/channels/console_video.c Thu Dec 20 06:56:07 2007
@@ -2987,18 +2987,6 @@
cleanup_sdl(env);
}
-/* see chan_oss.c for these macros */
-#ifndef M_START
-#define _UNDO_M_START
-#define M_START(var, val) \
- const char *__s = var; const char *__val = val;
-#define M_END(x) x;
-#define M_F(tag, f) if (!strcasecmp((__s), tag)) { f; } else
-#define M_BOOL(tag, dst) M_F(tag, (dst) = ast_true(__val) )
-#define M_UINT(tag, dst) M_F(tag, (dst) = strtoul(__val, NULL, 0) )
-#define M_STR(tag, dst) M_F(tag, ast_copy_string(dst, __val, sizeof(dst)))
-#endif
-
/*
* Parse a geometry string, accepting also common names for the formats.
* Trick: if we have a leading > or < and a numeric geometry,
@@ -3245,7 +3233,6 @@
const char *var, const char *val)
{
struct video_desc *env;
- M_START(var, val);
if (penv == NULL) {
ast_log(LOG_WARNING, "bad argument penv=NULL\n");
@@ -3267,31 +3254,25 @@
env->out.sendvideo = 1;
env->out.qmin = 3;
}
- M_STR("videodevice", env->out.videodevice)
- M_BOOL("sendvideo", env->out.sendvideo)
- M_F("video_size", video_geom(&env->out.enc_in, val))
- M_F("camera_size", video_geom(&env->out.loc_src, val))
- M_F("local_size", video_geom(&env->out.loc_dpy, val))
- M_F("remote_size", video_geom(&env->in.rem_dpy, val))
- M_STR("keypad", env->keypad_file)
- M_F("keypad_entry", keypad_cfg_read(&env->gui, val))
- M_STR("keypad_mask", env->keypad_mask)
- M_STR("keypad_font", env->keypad_font)
- M_UINT("fps", env->out.fps)
- M_UINT("bitrate", env->out.bitrate)
- M_UINT("qmin", env->out.qmin)
- M_STR("videocodec", env->codec_name)
- M_END(return 1;) /* the 'nothing found' case */
+ CV_START(var, val);
+ CV_STR("videodevice", env->out.videodevice);
+ CV_BOOL("sendvideo", env->out.sendvideo);
+ CV_F("video_size", video_geom(&env->out.enc_in, val));
+ CV_F("camera_size", video_geom(&env->out.loc_src, val));
+ CV_F("local_size", video_geom(&env->out.loc_dpy, val));
+ CV_F("remote_size", video_geom(&env->in.rem_dpy, val));
+ CV_STR("keypad", env->keypad_file);
+ CV_F("keypad_entry", keypad_cfg_read(&env->gui, val));
+ CV_STR("keypad_mask", env->keypad_mask);
+ CV_STR("keypad_font", env->keypad_font);
+ CV_UINT("fps", env->out.fps);
+ CV_UINT("bitrate", env->out.bitrate);
+ CV_UINT("qmin", env->out.qmin);
+ CV_STR("videocodec", env->codec_name);
+ return 1; /* nothing found */
+
+ CV_END; /* the 'nothing found' case */
return 0; /* found something */
}
-#ifdef _UNDO_M_START
-#undef M_START
-#undef M_END
-#undef M_F
-#undef M_BOOL
-#undef M_UINT
-#undef M_STR
-#undef _UNDO_M_START
-#endif
#endif /* video support */
Modified: trunk/include/asterisk/config.h
URL: http://svn.digium.com/view/asterisk/trunk/include/asterisk/config.h?view=diff&rev=94191&r1=94190&r2=94191
==============================================================================
--- trunk/include/asterisk/config.h (original)
+++ trunk/include/asterisk/config.h Thu Dec 20 06:56:07 2007
@@ -271,6 +271,7 @@
int config_text_file_save(const char *filename, const struct ast_config *cfg, const char *generator);
struct ast_config *ast_config_internal_load(const char *configfile, struct ast_config *cfg, struct ast_flags flags, const char *suggested_incl_file);
+
/*! \brief Support code to parse config file arguments
*
* The function ast_parse_arg() provides a generic interface to parse
@@ -358,6 +359,45 @@
int ast_parse_arg(const char *arg, enum ast_parse_flags flags,
void *result, ...);
+/*
+ * Parsing config file options in C is slightly annoying because we cannot use
+ * string in a switch() statement, yet we need a similar behaviour, with many
+ * branches and a break on a matching one.
+ * The following somehow simplifies the job: we create a block using
+ * the CV_START and CV_END macros, and then within the block we can run
+ * actions such as "if (condition) { body; break; }"
+ * Additional macros are present to run simple functions (e.g. ast_copy_string)
+ * or to pass arguments to ast_parse_arg()
+ *
+ * As an example:
+
+ CV_START(v->name, v->value); // start the block
+ CV_STR("foo", x_foo); // static string
+ CV_DSTR("bar", y_bar); // malloc'ed string
+ CV_F("bar", ...); // call a generic function
+ CV_END; // end the block
+ */
+
+/*! \brief the macro to open a block for variable parsing */
+#define CV_START(__in_var, __in_val) \
+ do { \
+ const char *__var = __in_var; \
+ const char *__val = __in_val;
+
+/*! \brief close a variable parsing block */
+#define CV_END } while (0)
+
+/*! \brief call a generic function if the name matches. */
+#define CV_F(__pattern, __body) if (!strcasecmp((__var), __pattern)) { __body; break; }
+
+/*! \brief helper macros to assign the value to a BOOL, UINT, static string and
+ * dynamic string
+ */
+#define CV_BOOL(__x, __dst) CV_F(__x, (__dst) = ast_true(__val) )
+#define CV_UINT(__x, __dst) CV_F(__x, (__dst) = strtoul(__val, NULL, 0) )
+#define CV_STR(__x, __dst) CV_F(__x, ast_copy_string(__dst, __val, sizeof(__dst)))
+#define CV_DSTR(__x, __dst) CV_F(__x, if (__dst) ast_free(__dst); __dst = ast_strdup(__val))
+
#if defined(__cplusplus) || defined(c_plusplus)
}
#endif
More information about the asterisk-commits
mailing list