[asterisk-commits] kpfleming: trunk r200656 - in /trunk: apps/ doc/ include/asterisk/

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Mon Jun 15 14:10:15 CDT 2009


Author: kpfleming
Date: Mon Jun 15 14:10:10 2009
New Revision: 200656

URL: http://svn.asterisk.org/svn-view/asterisk?view=rev&rev=200656
Log:
Last batch of 'static' qualifiers for module-level global variables.

Fix up modules in the 'apps' directory, and also correct the bad example of
enum definitions in include/asterisk/app.h, which many developers followed
(thanks for reading the documentation!). In addition, add some basic usage
examples of the 'pahole' and 'pglobal' tools to the coding guidelines.


Modified:
    trunk/apps/app_externalivr.c
    trunk/apps/app_fax.c
    trunk/apps/app_macro.c
    trunk/apps/app_minivm.c
    trunk/apps/app_mixmonitor.c
    trunk/apps/app_page.c
    trunk/apps/app_queue.c
    trunk/apps/app_read.c
    trunk/apps/app_readexten.c
    trunk/apps/app_skel.c
    trunk/apps/app_sms.c
    trunk/apps/app_stack.c
    trunk/apps/app_url.c
    trunk/apps/app_voicemail.c
    trunk/doc/CODING-GUIDELINES
    trunk/include/asterisk/app.h

Modified: trunk/apps/app_externalivr.c
URL: http://svn.asterisk.org/svn-view/asterisk/trunk/apps/app_externalivr.c?view=diff&rev=200656&r1=200655&r2=200656
==============================================================================
--- trunk/apps/app_externalivr.c (original)
+++ trunk/apps/app_externalivr.c Mon Jun 15 14:10:10 2009
@@ -95,11 +95,11 @@
 /* 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),

Modified: trunk/apps/app_fax.c
URL: http://svn.asterisk.org/svn-view/asterisk/trunk/apps/app_fax.c?view=diff&rev=200656&r1=200655&r2=200656
==============================================================================
--- trunk/apps/app_fax.c (original)
+++ trunk/apps/app_fax.c Mon Jun 15 14:10:10 2009
@@ -351,7 +351,7 @@
 	return 0;
 }
 
-struct ast_generator generator = {
+static struct ast_generator generator = {
 	alloc:		fax_generator_alloc,
 	generate: 	fax_generator_generate,
 };

Modified: trunk/apps/app_macro.c
URL: http://svn.asterisk.org/svn-view/asterisk/trunk/apps/app_macro.c?view=diff&rev=200656&r1=200655&r2=200656
==============================================================================
--- trunk/apps/app_macro.c (original)
+++ trunk/apps/app_macro.c Mon Jun 15 14:10:10 2009
@@ -159,7 +159,7 @@
 
 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,
 };

Modified: trunk/apps/app_minivm.c
URL: http://svn.asterisk.org/svn-view/asterisk/trunk/apps/app_minivm.c?view=diff&rev=200656&r1=200655&r2=200656
==============================================================================
--- trunk/apps/app_minivm.c (original)
+++ trunk/apps/app_minivm.c Mon Jun 15 14:10:10 2009
@@ -548,19 +548,19 @@
 
 
 
-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 @@
 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 */

Modified: trunk/apps/app_mixmonitor.c
URL: http://svn.asterisk.org/svn-view/asterisk/trunk/apps/app_mixmonitor.c?view=diff&rev=200656&r1=200655&r2=200656
==============================================================================
--- trunk/apps/app_mixmonitor.c (original)
+++ trunk/apps/app_mixmonitor.c Mon Jun 15 14:10:10 2009
@@ -132,8 +132,6 @@
 
 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 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),

Modified: trunk/apps/app_page.c
URL: http://svn.asterisk.org/svn-view/asterisk/trunk/apps/app_page.c?view=diff&rev=200656&r1=200655&r2=200656
==============================================================================
--- trunk/apps/app_page.c (original)
+++ trunk/apps/app_page.c Mon Jun 15 14:10:10 2009
@@ -101,13 +101,13 @@
  ***/
 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),

Modified: trunk/apps/app_queue.c
URL: http://svn.asterisk.org/svn-view/asterisk/trunk/apps/app_queue.c?view=diff&rev=200656&r1=200655&r2=200656
==============================================================================
--- trunk/apps/app_queue.c (original)
+++ trunk/apps/app_queue.c Mon Jun 15 14:10:10 2009
@@ -977,7 +977,7 @@
 	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;
 

Modified: trunk/apps/app_read.c
URL: http://svn.asterisk.org/svn-view/asterisk/trunk/apps/app_read.c?view=diff&rev=200656&r1=200655&r2=200656
==============================================================================
--- trunk/apps/app_read.c (original)
+++ trunk/apps/app_read.c Mon Jun 15 14:10:10 2009
@@ -108,11 +108,11 @@
 	</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),

Modified: trunk/apps/app_readexten.c
URL: http://svn.asterisk.org/svn-view/asterisk/trunk/apps/app_readexten.c?view=diff&rev=200656&r1=200655&r2=200656
==============================================================================
--- trunk/apps/app_readexten.c (original)
+++ trunk/apps/app_readexten.c Mon Jun 15 14:10:10 2009
@@ -111,11 +111,11 @@
 	</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),

Modified: trunk/apps/app_skel.c
URL: http://svn.asterisk.org/svn-view/asterisk/trunk/apps/app_skel.c?view=diff&rev=200656&r1=200655&r2=200656
==============================================================================
--- trunk/apps/app_skel.c (original)
+++ trunk/apps/app_skel.c Mon Jun 15 14:10:10 2009
@@ -74,18 +74,18 @@
 
 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),

Modified: trunk/apps/app_sms.c
URL: http://svn.asterisk.org/svn-view/asterisk/trunk/apps/app_sms.c?view=diff&rev=200656&r1=200655&r2=200656
==============================================================================
--- trunk/apps/app_sms.c (original)
+++ trunk/apps/app_sms.c Mon Jun 15 14:10:10 2009
@@ -1832,19 +1832,19 @@
  *	- 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),

Modified: trunk/apps/app_stack.c
URL: http://svn.asterisk.org/svn-view/asterisk/trunk/apps/app_stack.c?view=diff&rev=200656&r1=200655&r2=200656
==============================================================================
--- trunk/apps/app_stack.c (original)
+++ trunk/apps/app_stack.c Mon Jun 15 14:10:10 2009
@@ -642,7 +642,7 @@
 	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)

Modified: trunk/apps/app_url.c
URL: http://svn.asterisk.org/svn-view/asterisk/trunk/apps/app_url.c?view=diff&rev=200656&r1=200655&r2=200656
==============================================================================
--- trunk/apps/app_url.c (original)
+++ trunk/apps/app_url.c Mon Jun 15 14:10:10 2009
@@ -82,9 +82,9 @@
 
 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),

Modified: trunk/apps/app_voicemail.c
URL: http://svn.asterisk.org/svn-view/asterisk/trunk/apps/app_voicemail.c?view=diff&rev=200656&r1=200655&r2=200656
==============================================================================
--- trunk/apps/app_voicemail.c (original)
+++ trunk/apps/app_voicemail.c Mon Jun 15 14:10:10 2009
@@ -425,16 +425,16 @@
 #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 @@
 	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),

Modified: trunk/doc/CODING-GUIDELINES
URL: http://svn.asterisk.org/svn-view/asterisk/trunk/doc/CODING-GUIDELINES?view=diff&rev=200656&r1=200655&r2=200656
==============================================================================
--- trunk/doc/CODING-GUIDELINES (original)
+++ trunk/doc/CODING-GUIDELINES Mon Jun 15 14:10:10 2009
@@ -114,7 +114,6 @@
 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 @@
   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
 ----------------------

Modified: trunk/include/asterisk/app.h
URL: http://svn.asterisk.org/svn-view/asterisk/trunk/include/asterisk/app.h?view=diff&rev=200656&r1=200655&r2=200656
==============================================================================
--- trunk/include/asterisk/app.h (original)
+++ trunk/include/asterisk/app.h Mon Jun 15 14:10:10 2009
@@ -447,19 +447,19 @@
 
   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),




More information about the asterisk-commits mailing list