diff options
-rw-r--r-- | apps/app_externalivr.c | 4 | ||||
-rw-r--r-- | apps/app_fax.c | 2 | ||||
-rw-r--r-- | apps/app_macro.c | 2 | ||||
-rw-r--r-- | apps/app_minivm.c | 10 | ||||
-rw-r--r-- | apps/app_mixmonitor.c | 10 | ||||
-rw-r--r-- | apps/app_page.c | 4 | ||||
-rw-r--r-- | apps/app_queue.c | 2 | ||||
-rw-r--r-- | apps/app_read.c | 4 | ||||
-rw-r--r-- | apps/app_readexten.c | 4 | ||||
-rw-r--r-- | apps/app_skel.c | 8 | ||||
-rw-r--r-- | apps/app_sms.c | 8 | ||||
-rw-r--r-- | apps/app_stack.c | 2 | ||||
-rw-r--r-- | apps/app_url.c | 4 | ||||
-rw-r--r-- | apps/app_voicemail.c | 12 | ||||
-rw-r--r-- | doc/CODING-GUIDELINES | 39 | ||||
-rw-r--r-- | include/asterisk/app.h | 8 |
16 files changed, 76 insertions, 47 deletions
diff --git a/apps/app_externalivr.c b/apps/app_externalivr.c index 3a8f0a95d..3a3644923 100644 --- a/apps/app_externalivr.c +++ b/apps/app_externalivr.c @@ -95,11 +95,11 @@ static const char app[] = "ExternalIVR"; /* XXX the parser in gcc 2.95 gets confused if you don't put a space between 'name' and the comma */ #define ast_chan_log(level, channel, format, ...) ast_log(level, "%s: " format, channel->name , ## __VA_ARGS__) -enum { +enum options_flags { noanswer = (1 << 0), ignore_hangup = (1 << 1), run_dead = (1 << 2), -} options_flags; +}; AST_APP_OPTIONS(app_opts, { AST_APP_OPTION('n', noanswer), diff --git a/apps/app_fax.c b/apps/app_fax.c index 8f98af2e7..bd6bdf9f6 100644 --- a/apps/app_fax.c +++ b/apps/app_fax.c @@ -351,7 +351,7 @@ static int fax_generator_generate(struct ast_channel *chan, void *data, int len, return 0; } -struct ast_generator generator = { +static struct ast_generator generator = { alloc: fax_generator_alloc, generate: fax_generator_generate, }; diff --git a/apps/app_macro.c b/apps/app_macro.c index f3a6b546e..e48d3c361 100644 --- a/apps/app_macro.c +++ b/apps/app_macro.c @@ -159,7 +159,7 @@ static char *exit_app = "MacroExit"; static void macro_fixup(void *data, struct ast_channel *old_chan, struct ast_channel *new_chan); -struct ast_datastore_info macro_ds_info = { +static struct ast_datastore_info macro_ds_info = { .type = "MACRO", .chan_fixup = macro_fixup, }; diff --git a/apps/app_minivm.c b/apps/app_minivm.c index afbd9dc2e..ac48aa50f 100644 --- a/apps/app_minivm.c +++ b/apps/app_minivm.c @@ -548,19 +548,19 @@ static char *app_minivm_mwi = "MinivmMWI"; -enum { +enum minivm_option_flags { OPT_SILENT = (1 << 0), OPT_BUSY_GREETING = (1 << 1), OPT_UNAVAIL_GREETING = (1 << 2), OPT_TEMP_GREETING = (1 << 3), OPT_NAME_GREETING = (1 << 4), OPT_RECORDGAIN = (1 << 5), -} minivm_option_flags; +}; -enum { +enum minivm_option_args { OPT_ARG_RECORDGAIN = 0, OPT_ARG_ARRAY_SIZE = 1, -} minivm_option_args; +}; AST_APP_OPTIONS(minivm_app_options, { AST_APP_OPTION('s', OPT_SILENT), @@ -670,7 +670,7 @@ static struct minivm_stats global_stats; AST_MUTEX_DEFINE_STATIC(minivmlock); /*!< Lock to protect voicemail system */ AST_MUTEX_DEFINE_STATIC(minivmloglock); /*!< Lock to protect voicemail system log file */ -FILE *minivmlogfile; /*!< The minivm log file */ +static FILE *minivmlogfile; /*!< The minivm log file */ static int global_vmminmessage; /*!< Minimum duration of messages */ static int global_vmmaxmessage; /*!< Maximum duration of message */ diff --git a/apps/app_mixmonitor.c b/apps/app_mixmonitor.c index d6e4ed044..d4b8519cb 100644 --- a/apps/app_mixmonitor.c +++ b/apps/app_mixmonitor.c @@ -132,8 +132,6 @@ static const char * const app = "MixMonitor"; static const char * const stop_app = "StopMixMonitor"; -struct module_symbols *me; - static const char * const mixmonitor_spy_type = "MixMonitor"; struct mixmonitor { @@ -145,20 +143,20 @@ struct mixmonitor { struct ast_autochan *autochan; }; -enum { +enum mixmonitor_flags { MUXFLAG_APPEND = (1 << 1), MUXFLAG_BRIDGED = (1 << 2), MUXFLAG_VOLUME = (1 << 3), MUXFLAG_READVOLUME = (1 << 4), MUXFLAG_WRITEVOLUME = (1 << 5), -} mixmonitor_flags; +}; -enum { +enum mixmonitor_args { OPT_ARG_READVOLUME = 0, OPT_ARG_WRITEVOLUME, OPT_ARG_VOLUME, OPT_ARG_ARRAY_SIZE, -} mixmonitor_args; +}; AST_APP_OPTIONS(mixmonitor_opts, { AST_APP_OPTION('a', MUXFLAG_APPEND), diff --git a/apps/app_page.c b/apps/app_page.c index b277dab56..04f9f98c5 100644 --- a/apps/app_page.c +++ b/apps/app_page.c @@ -101,13 +101,13 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$") ***/ static const char * const app_page= "Page"; -enum { +enum page_opt_flags { PAGE_DUPLEX = (1 << 0), PAGE_QUIET = (1 << 1), PAGE_RECORD = (1 << 2), PAGE_SKIP = (1 << 3), PAGE_IGNORE_FORWARDS = (1 << 4), -} page_opt_flags; +}; AST_APP_OPTIONS(page_opts, { AST_APP_OPTION('d', PAGE_DUPLEX), diff --git a/apps/app_queue.c b/apps/app_queue.c index e32caa78d..bebe543b8 100644 --- a/apps/app_queue.c +++ b/apps/app_queue.c @@ -977,7 +977,7 @@ struct rule_list { AST_LIST_ENTRY(rule_list) list; }; -AST_LIST_HEAD_STATIC(rule_lists, rule_list); +static AST_LIST_HEAD_STATIC(rule_lists, rule_list); static struct ao2_container *queues; diff --git a/apps/app_read.c b/apps/app_read.c index 3e22d2ef8..a5e4a6434 100644 --- a/apps/app_read.c +++ b/apps/app_read.c @@ -108,11 +108,11 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$") </application> ***/ -enum { +enum read_option_flags { OPT_SKIP = (1 << 0), OPT_INDICATION = (1 << 1), OPT_NOANSWER = (1 << 2), -} read_option_flags; +}; AST_APP_OPTIONS(read_app_options, { AST_APP_OPTION('s', OPT_SKIP), diff --git a/apps/app_readexten.c b/apps/app_readexten.c index ef59aa538..b6b6305a5 100644 --- a/apps/app_readexten.c +++ b/apps/app_readexten.c @@ -111,11 +111,11 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$") </function> ***/ -enum { +enum readexten_option_flags { OPT_SKIP = (1 << 0), OPT_INDICATION = (1 << 1), OPT_NOANSWER = (1 << 2), -} readexten_option_flags; +}; AST_APP_OPTIONS(readexten_app_options, { AST_APP_OPTION('s', OPT_SKIP), diff --git a/apps/app_skel.c b/apps/app_skel.c index 1796ab4f5..c58d451e4 100644 --- a/apps/app_skel.c +++ b/apps/app_skel.c @@ -74,18 +74,18 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$") static char *app = "Skel"; -enum { +enum option_flags { OPTION_A = (1 << 0), OPTION_B = (1 << 1), OPTION_C = (1 << 2), -} option_flags; +}; -enum { +enum option_args { OPTION_ARG_B = 0, OPTION_ARG_C = 1, /* This *must* be the last value in this enum! */ OPTION_ARG_ARRAY_SIZE = 2, -} option_args; +}; AST_APP_OPTIONS(app_opts,{ AST_APP_OPTION('a', OPTION_A), diff --git a/apps/app_sms.c b/apps/app_sms.c index a0e0c2fc0..78af84926 100644 --- a/apps/app_sms.c +++ b/apps/app_sms.c @@ -1832,19 +1832,19 @@ static void sms_process(sms_t * h, int samples, signed short *data) * - AST_APP_OPTIONS() to drive the parsing routine * - in the function, AST_DECLARE_APP_ARGS(...) for the arguments. */ -enum { +enum sms_flags { OPTION_BE_SMSC = (1 << 0), /* act as sms center */ OPTION_ANSWER = (1 << 1), /* answer on incoming calls */ OPTION_TWO = (1 << 2), /* Use Protocol Two */ OPTION_PAUSE = (1 << 3), /* pause before sending data, in ms */ OPTION_SRR = (1 << 4), /* set srr */ OPTION_DCS = (1 << 5), /* set dcs */ -} sms_flags; +}; -enum { +enum sms_opt_args { OPTION_ARG_PAUSE = 0, OPTION_ARG_ARRAY_SIZE -} sms_opt_args; +}; AST_APP_OPTIONS(sms_options, { AST_APP_OPTION('s', OPTION_BE_SMSC), diff --git a/apps/app_stack.c b/apps/app_stack.c index 100a59603..84be40ba2 100644 --- a/apps/app_stack.c +++ b/apps/app_stack.c @@ -642,7 +642,7 @@ static int handle_gosub(struct ast_channel *chan, AGI *agi, int argc, const char return RESULT_SUCCESS; } -struct agi_command gosub_agi_command = +static struct agi_command gosub_agi_command = { { "gosub", NULL }, handle_gosub, NULL, NULL, 0 }; static int unload_module(void) diff --git a/apps/app_url.c b/apps/app_url.c index 234cff619..a36daa26b 100644 --- a/apps/app_url.c +++ b/apps/app_url.c @@ -82,9 +82,9 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$") static char *app = "SendURL"; -enum { +enum option_flags { OPTION_WAIT = (1 << 0), -} option_flags; +}; AST_APP_OPTIONS(app_opts,{ AST_APP_OPTION('w', OPTION_WAIT), diff --git a/apps/app_voicemail.c b/apps/app_voicemail.c index 1c14f2e4f..2655fc2ab 100644 --- a/apps/app_voicemail.c +++ b/apps/app_voicemail.c @@ -425,16 +425,16 @@ static AST_LIST_HEAD_STATIC(vmstates, vmstate); #define ERROR_LOCK_PATH -100 -enum { +enum vm_box { NEW_FOLDER, OLD_FOLDER, WORK_FOLDER, FAMILY_FOLDER, FRIENDS_FOLDER, GREETINGS_FOLDER -} vm_box; +}; -enum { +enum vm_option_flags { OPT_SILENT = (1 << 0), OPT_BUSY_GREETING = (1 << 1), OPT_UNAVAIL_GREETING = (1 << 2), @@ -444,15 +444,15 @@ enum { OPT_DTMFEXIT = (1 << 7), OPT_MESSAGE_Urgent = (1 << 8), OPT_MESSAGE_PRIORITY = (1 << 9) -} vm_option_flags; +}; -enum { +enum vm_option_args { OPT_ARG_RECORDGAIN = 0, OPT_ARG_PLAYFOLDER = 1, OPT_ARG_DTMFEXIT = 2, /* This *must* be the last value in this enum! */ OPT_ARG_ARRAY_SIZE = 3, -} vm_option_args; +}; AST_APP_OPTIONS(vm_app_options, { AST_APP_OPTION('s', OPT_SILENT), diff --git a/doc/CODING-GUIDELINES b/doc/CODING-GUIDELINES index 5b7e75893..80ab20767 100644 --- a/doc/CODING-GUIDELINES +++ b/doc/CODING-GUIDELINES @@ -114,7 +114,6 @@ those you really need. Apart from obvious cases (e.g. module.h which is almost always necessary) write a short comment next to each #include to explain why you need it. - * Declaration of functions and variables ---------------------------------------- @@ -122,14 +121,46 @@ explain why you need it. since it is harder to read and not portable to GCC 2.95 and others. - Functions and variables that are not intended to be used outside the module - must be declared static. + must be declared static. If you are compiling on a Linux platform that has the + 'dwarves' package available, you can use the 'pglobal' tool from that package + to check for unintended global variables or functions being exposed in your + object files. Usage is very simple: + + $ pglobal -vf <path to .o file> - When reading integer numeric input with scanf (or variants), do _NOT_ use '%i' unless you specifically want to allow non-base-10 input; '%d' is always a better choice, since it will not silently turn numbers with leading zeros into base-8. -- Strings that are coming from input should not be used as a first argument to - a formatted *printf function. +- Strings that are coming from input should not be used as the format argument to + any printf-style function. + +* Structure alignment and padding +--------------------------------- + +On many platforms, structure fields (in structures that are not marked 'packed') +will be laid out by the compiler with gaps (padding) between them, in order to +satisfy alignment requirements. As a simple example: + +struct foo { + int bar; + void *xyz; +} + +On nearly every 64-bit platform, this will result in 4 bytes of dead space between +'bar' and 'xyz', because pointers on 64-bit platforms must be aligned on 8-byte +boundaries. Once you have your code written and tested, it may be worthwhile to review +your structure definitions to look for problems of this nature. If you are on a Linux +platform with the 'dwarves' package available, the 'pahole' tool from that package +can be used to both check for padding issues of this type and also propose reorganized +structure definitions to eliminate it. Usage is quite simple; for a structure named 'foo', +the command would look something like this: + +$ pahole --reorganize --show_reorg_steps -C foo <path to module> + +The 'pahole' tool has many other modes available, including some that will list all the +structures declared in the module and the amount of padding in each one that could possibly +be recovered. * Use the internal API ---------------------- diff --git a/include/asterisk/app.h b/include/asterisk/app.h index da947b5d7..3bf780060 100644 --- a/include/asterisk/app.h +++ b/include/asterisk/app.h @@ -447,19 +447,19 @@ struct ast_app_option { Example usage: \code - enum { + enum my_app_option_flags { OPT_JUMP = (1 << 0), OPT_BLAH = (1 << 1), OPT_BLORT = (1 << 2), - } my_app_option_flags; + }; - enum { + enum my_app_option_args { OPT_ARG_BLAH = 0, OPT_ARG_BLORT, !! this entry tells how many possible arguments there are, and must be the last entry in the list OPT_ARG_ARRAY_SIZE, - } my_app_option_args; + }; AST_APP_OPTIONS(my_app_options, { AST_APP_OPTION('j', OPT_JUMP), |