[PATCH] Combine Globals and Locals into a single Variables structure that parallels Defaults, shortening some code. Improve comments and make other minor cleanups.

Matt McCutchen matt at mattmccutchen.net
Sat Jan 31 05:53:32 GMT 2009


---

This is the first refactoring I mentioned.  I decided not to inline
global_vars because I find it helpful to see the global parameters in
their own structure.

 loadparm.c |  157 ++++++++++++++++++++++++++++++-----------------------------
 1 files changed, 80 insertions(+), 77 deletions(-)

diff --git a/loadparm.c b/loadparm.c
index 2a9d8b4..28a11d1 100644
--- a/loadparm.c
+++ b/loadparm.c
@@ -87,7 +87,7 @@ struct parm_struct {
 /* some helpful bits */
 #define iSECTION(i) ((local_vars*)section_list.items)[i]
 #define LP_SNUM_OK(i) ((i) >= 0 && (i) < (int)section_list.count)
-#define SECTION_PTR(s, p) (((char*)(s)) + (ptrdiff_t)(((char*)(p))-(char*)&Locals))
+#define SECTION_PTR(s, p) (((char*)(s)) + (ptrdiff_t)(((char*)(p))-(char*)&Variables.l))
 
 /*
  * This structure describes global (ie., server-wide) parameters.
@@ -101,8 +101,6 @@ typedef struct {
 	int rsync_port;
 } global_vars;
 
-static global_vars Globals;
-
 /*
  * This structure describes a single section.  Their order must match the
  * initializers below, which you can accomplish by keeping each sub-section
@@ -154,15 +152,18 @@ typedef struct {
 	BOOL write_only;
 } local_vars;
 
-static local_vars Locals;
-
+/* This structure describes the global variables (g) as well as the globally
+ * specified values of the local variables (l), which are used when modules
+ * don't specify their own values. */
 typedef struct {
 	global_vars g;
 	local_vars l;
 } all_vars;
 
-/* This is used to reset all values before a config read.  In order
- * to make these easy to keep sorted in the same way as the variables
+/* The application defaults for all the variables.  "Variables" is
+ * re-initialized from "Defaults" just before each config read.
+ *
+ * In order to make these easy to keep sorted in the same way as the variables
  * above, use the variable name in the leading comment, including a
  * trailing ';' (to avoid a sorting problem with trailing digits). */
 static const all_vars Defaults = {
@@ -224,9 +225,14 @@ static const all_vars Defaults = {
  }
 };
 
+/* The currently configured values for all the variables. */
+all_vars Variables;
+
+/* Stack of "Variables" states for the &include directive. */
+static item_list Variables_stack = EMPTY_ITEM_LIST;
+
 /* local variables */
 static item_list section_list = EMPTY_ITEM_LIST;
-static item_list section_stack = EMPTY_ITEM_LIST;
 static int iSectionIndex = -1;
 static BOOL bInGlobalSection = True;
 
@@ -301,63 +307,62 @@ static struct enum_list enum_facilities[] = {
 
 static struct parm_struct parm_table[] =
 {
- {"address",           P_STRING, P_GLOBAL,&Globals.bind_address,       NULL,0},
- {"motd file",         P_STRING, P_GLOBAL,&Globals.motd_file,          NULL,0},
- {"pid file",          P_STRING, P_GLOBAL,&Globals.pid_file,           NULL,0},
- {"port",              P_INTEGER,P_GLOBAL,&Globals.rsync_port,         NULL,0},
- {"socket options",    P_STRING, P_GLOBAL,&Globals.socket_options,     NULL,0},
-
- {"auth users",        P_STRING, P_LOCAL, &Locals.auth_users,          NULL,0},
- {"charset",           P_STRING, P_LOCAL, &Locals.charset,             NULL,0},
- {"comment",           P_STRING, P_LOCAL, &Locals.comment,             NULL,0},
- {"dont compress",     P_STRING, P_LOCAL, &Locals.dont_compress,       NULL,0},
- {"exclude from",      P_STRING, P_LOCAL, &Locals.exclude_from,        NULL,0},
- {"exclude",           P_STRING, P_LOCAL, &Locals.exclude,             NULL,0},
- {"fake super",        P_BOOL,   P_LOCAL, &Locals.fake_super,          NULL,0},
- {"filter",            P_STRING, P_LOCAL, &Locals.filter,              NULL,0},
- {"gid",               P_STRING, P_LOCAL, &Locals.gid,                 NULL,0},
- {"hosts allow",       P_STRING, P_LOCAL, &Locals.hosts_allow,         NULL,0},
- {"hosts deny",        P_STRING, P_LOCAL, &Locals.hosts_deny,          NULL,0},
- {"ignore errors",     P_BOOL,   P_LOCAL, &Locals.ignore_errors,       NULL,0},
- {"ignore nonreadable",P_BOOL,   P_LOCAL, &Locals.ignore_nonreadable,  NULL,0},
- {"include from",      P_STRING, P_LOCAL, &Locals.include_from,        NULL,0},
- {"include",           P_STRING, P_LOCAL, &Locals.include,             NULL,0},
- {"incoming chmod",    P_STRING, P_LOCAL, &Locals.incoming_chmod,      NULL,0},
- {"list",              P_BOOL,   P_LOCAL, &Locals.list,                NULL,0},
- {"lock file",         P_STRING, P_LOCAL, &Locals.lock_file,           NULL,0},
- {"log file",          P_STRING, P_LOCAL, &Locals.log_file,            NULL,0},
- {"log format",        P_STRING, P_LOCAL, &Locals.log_format,          NULL,0},
- {"max connections",   P_INTEGER,P_LOCAL, &Locals.max_connections,     NULL,0},
- {"max verbosity",     P_INTEGER,P_LOCAL, &Locals.max_verbosity,       NULL,0},
- {"munge symlinks",    P_BOOL,   P_LOCAL, &Locals.munge_symlinks,      NULL,0},
- {"name",              P_STRING, P_LOCAL, &Locals.name,                NULL,0},
- {"numeric ids",       P_BOOL,   P_LOCAL, &Locals.numeric_ids,         NULL,0},
- {"outgoing chmod",    P_STRING, P_LOCAL, &Locals.outgoing_chmod,      NULL,0},
- {"path",              P_PATH,   P_LOCAL, &Locals.path,                NULL,0},
+ {"address",           P_STRING, P_GLOBAL,&Variables.g.bind_address,       NULL,0},
+ {"motd file",         P_STRING, P_GLOBAL,&Variables.g.motd_file,          NULL,0},
+ {"pid file",          P_STRING, P_GLOBAL,&Variables.g.pid_file,           NULL,0},
+ {"port",              P_INTEGER,P_GLOBAL,&Variables.g.rsync_port,         NULL,0},
+ {"socket options",    P_STRING, P_GLOBAL,&Variables.g.socket_options,     NULL,0},
+
+ {"auth users",        P_STRING, P_LOCAL, &Variables.l.auth_users,          NULL,0},
+ {"charset",           P_STRING, P_LOCAL, &Variables.l.charset,             NULL,0},
+ {"comment",           P_STRING, P_LOCAL, &Variables.l.comment,             NULL,0},
+ {"dont compress",     P_STRING, P_LOCAL, &Variables.l.dont_compress,       NULL,0},
+ {"exclude from",      P_STRING, P_LOCAL, &Variables.l.exclude_from,        NULL,0},
+ {"exclude",           P_STRING, P_LOCAL, &Variables.l.exclude,             NULL,0},
+ {"fake super",        P_BOOL,   P_LOCAL, &Variables.l.fake_super,          NULL,0},
+ {"filter",            P_STRING, P_LOCAL, &Variables.l.filter,              NULL,0},
+ {"gid",               P_STRING, P_LOCAL, &Variables.l.gid,                 NULL,0},
+ {"hosts allow",       P_STRING, P_LOCAL, &Variables.l.hosts_allow,         NULL,0},
+ {"hosts deny",        P_STRING, P_LOCAL, &Variables.l.hosts_deny,          NULL,0},
+ {"ignore errors",     P_BOOL,   P_LOCAL, &Variables.l.ignore_errors,       NULL,0},
+ {"ignore nonreadable",P_BOOL,   P_LOCAL, &Variables.l.ignore_nonreadable,  NULL,0},
+ {"include from",      P_STRING, P_LOCAL, &Variables.l.include_from,        NULL,0},
+ {"include",           P_STRING, P_LOCAL, &Variables.l.include,             NULL,0},
+ {"incoming chmod",    P_STRING, P_LOCAL, &Variables.l.incoming_chmod,      NULL,0},
+ {"list",              P_BOOL,   P_LOCAL, &Variables.l.list,                NULL,0},
+ {"lock file",         P_STRING, P_LOCAL, &Variables.l.lock_file,           NULL,0},
+ {"log file",          P_STRING, P_LOCAL, &Variables.l.log_file,            NULL,0},
+ {"log format",        P_STRING, P_LOCAL, &Variables.l.log_format,          NULL,0},
+ {"max connections",   P_INTEGER,P_LOCAL, &Variables.l.max_connections,     NULL,0},
+ {"max verbosity",     P_INTEGER,P_LOCAL, &Variables.l.max_verbosity,       NULL,0},
+ {"munge symlinks",    P_BOOL,   P_LOCAL, &Variables.l.munge_symlinks,      NULL,0},
+ {"name",              P_STRING, P_LOCAL, &Variables.l.name,                NULL,0},
+ {"numeric ids",       P_BOOL,   P_LOCAL, &Variables.l.numeric_ids,         NULL,0},
+ {"outgoing chmod",    P_STRING, P_LOCAL, &Variables.l.outgoing_chmod,      NULL,0},
+ {"path",              P_PATH,   P_LOCAL, &Variables.l.path,                NULL,0},
 #ifdef HAVE_PUTENV
- {"post-xfer exec",    P_STRING, P_LOCAL, &Locals.postxfer_exec,       NULL,0},
- {"pre-xfer exec",     P_STRING, P_LOCAL, &Locals.prexfer_exec,        NULL,0},
+ {"post-xfer exec",    P_STRING, P_LOCAL, &Variables.l.postxfer_exec,       NULL,0},
+ {"pre-xfer exec",     P_STRING, P_LOCAL, &Variables.l.prexfer_exec,        NULL,0},
 #endif
- {"read only",         P_BOOL,   P_LOCAL, &Locals.read_only,           NULL,0},
- {"refuse options",    P_STRING, P_LOCAL, &Locals.refuse_options,      NULL,0},
- {"reverse lookup",    P_BOOL,   P_LOCAL, &Locals.reverse_lookup,      NULL,0},
- {"secrets file",      P_STRING, P_LOCAL, &Locals.secrets_file,        NULL,0},
- {"strict modes",      P_BOOL,   P_LOCAL, &Locals.strict_modes,        NULL,0},
- {"syslog facility",   P_ENUM,   P_LOCAL, &Locals.syslog_facility,     enum_facilities,0},
- {"temp dir",          P_PATH,   P_LOCAL, &Locals.temp_dir,            NULL,0},
- {"timeout",           P_INTEGER,P_LOCAL, &Locals.timeout,             NULL,0},
- {"transfer logging",  P_BOOL,   P_LOCAL, &Locals.transfer_logging,    NULL,0},
- {"uid",               P_STRING, P_LOCAL, &Locals.uid,                 NULL,0},
- {"use chroot",        P_BOOL,   P_LOCAL, &Locals.use_chroot,          NULL,0},
- {"write only",        P_BOOL,   P_LOCAL, &Locals.write_only,          NULL,0},
+ {"read only",         P_BOOL,   P_LOCAL, &Variables.l.read_only,           NULL,0},
+ {"refuse options",    P_STRING, P_LOCAL, &Variables.l.refuse_options,      NULL,0},
+ {"reverse lookup",    P_BOOL,   P_LOCAL, &Variables.l.reverse_lookup,      NULL,0},
+ {"secrets file",      P_STRING, P_LOCAL, &Variables.l.secrets_file,        NULL,0},
+ {"strict modes",      P_BOOL,   P_LOCAL, &Variables.l.strict_modes,        NULL,0},
+ {"syslog facility",   P_ENUM,   P_LOCAL, &Variables.l.syslog_facility,     enum_facilities,0},
+ {"temp dir",          P_PATH,   P_LOCAL, &Variables.l.temp_dir,            NULL,0},
+ {"timeout",           P_INTEGER,P_LOCAL, &Variables.l.timeout,             NULL,0},
+ {"transfer logging",  P_BOOL,   P_LOCAL, &Variables.l.transfer_logging,    NULL,0},
+ {"uid",               P_STRING, P_LOCAL, &Variables.l.uid,                 NULL,0},
+ {"use chroot",        P_BOOL,   P_LOCAL, &Variables.l.use_chroot,          NULL,0},
+ {"write only",        P_BOOL,   P_LOCAL, &Variables.l.write_only,          NULL,0},
  {NULL,                P_BOOL,   P_NONE,  NULL,                        NULL,0}
 };
 
 /* Initialise the Default all_vars structure. */
 static void reset_all_vars(void)
 {
-	memcpy(&Globals, &Defaults.g, sizeof Globals);
-	memcpy(&Locals, &Defaults.l, sizeof Locals);
+	memcpy(&Variables, &Defaults, sizeof (all_vars));
 }
 
 /* In this section all the functions that are used to access the
@@ -373,20 +378,20 @@ static void reset_all_vars(void)
  int fn_name(void) {return *(int *)(ptr);}
 
 #define FN_LOCAL_STRING(fn_name, val) \
- char *fn_name(int i) {return LP_SNUM_OK(i) && iSECTION(i).val? iSECTION(i).val : (Locals.val? Locals.val : "");}
+ char *fn_name(int i) {return LP_SNUM_OK(i) && iSECTION(i).val? iSECTION(i).val : (Variables.l.val? Variables.l.val : "");}
 #define FN_LOCAL_BOOL(fn_name, val) \
- BOOL fn_name(int i) {return LP_SNUM_OK(i)? iSECTION(i).val : Locals.val;}
+ BOOL fn_name(int i) {return LP_SNUM_OK(i)? iSECTION(i).val : Variables.l.val;}
 #define FN_LOCAL_CHAR(fn_name, val) \
- char fn_name(int i) {return LP_SNUM_OK(i)? iSECTION(i).val : Locals.val;}
+ char fn_name(int i) {return LP_SNUM_OK(i)? iSECTION(i).val : Variables.l.val;}
 #define FN_LOCAL_INTEGER(fn_name, val) \
- int fn_name(int i) {return LP_SNUM_OK(i)? iSECTION(i).val : Locals.val;}
+ int fn_name(int i) {return LP_SNUM_OK(i)? iSECTION(i).val : Variables.l.val;}
 
-FN_GLOBAL_STRING(lp_bind_address, &Globals.bind_address)
-FN_GLOBAL_STRING(lp_motd_file, &Globals.motd_file)
-FN_GLOBAL_STRING(lp_pid_file, &Globals.pid_file)
-FN_GLOBAL_STRING(lp_socket_options, &Globals.socket_options)
+FN_GLOBAL_STRING(lp_bind_address, &Variables.g.bind_address)
+FN_GLOBAL_STRING(lp_motd_file, &Variables.g.motd_file)
+FN_GLOBAL_STRING(lp_pid_file, &Variables.g.pid_file)
+FN_GLOBAL_STRING(lp_socket_options, &Variables.g.socket_options)
 
-FN_GLOBAL_INTEGER(lp_rsync_port, &Globals.rsync_port)
+FN_GLOBAL_INTEGER(lp_rsync_port, &Variables.g.rsync_port)
 
 FN_LOCAL_STRING(lp_auth_users, auth_users)
 FN_LOCAL_STRING(lp_charset, charset)
@@ -439,7 +444,7 @@ FN_LOCAL_BOOL(lp_write_only, write_only)
  * FIXME There is a small leak here in that sometimes the existing
  * value will be dynamically allocated, and the old copy is lost.
  * However, we can't always deallocate the old value, because in the
- * case of Locals, it points to a static string.  It would be nice
+ * case of Variables.l, it points to a static string.  It would be nice
  * to have either all-strdup'd values, or to never need to free
  * memory. */
 static void string_set(char **s, const char *v)
@@ -494,8 +499,8 @@ static void copy_section(local_vars *psectionDest, local_vars *psectionSource)
 /* Initialise a section to the defaults. */
 static void init_section(local_vars *psection)
 {
-	memset(psection, 0, sizeof Locals);
-	copy_section(psection, &Locals);
+	memset(psection, 0, sizeof (local_vars));
+	copy_section(psection, &Variables.l);
 }
 
 /* Do a case-insensitive, whitespace-ignoring string compare. */
@@ -689,18 +694,16 @@ static BOOL do_section(char *sectionname)
 	if (*sectionname == ']') { /* A special push/pop/reset directive from params.c */
 		bInGlobalSection = 1;
 		if (strcmp(sectionname+1, "push") == 0) {
-			all_vars *vp = EXPAND_ITEM_LIST(&section_stack, all_vars, 2);
-			memcpy(&vp->g, &Globals, sizeof Globals);
-			memcpy(&vp->l, &Locals, sizeof Locals);
+			all_vars *vp = EXPAND_ITEM_LIST(&Variables_stack, all_vars, 2);
+			memcpy(vp, &Variables, sizeof (all_vars));
 		} else if (strcmp(sectionname+1, "pop") == 0
 		 || strcmp(sectionname+1, "reset") == 0) {
-			all_vars *vp = ((all_vars*)section_stack.items) + section_stack.count - 1;
-			if (!section_stack.count)
+			if (!Variables_stack.count)
 				return False;
-			memcpy(&Globals, &vp->g, sizeof Globals);
-			memcpy(&Locals, &vp->l, sizeof Locals);
+			all_vars *vp = ((all_vars*)Variables_stack.items) + Variables_stack.count - 1;
+			memcpy(&Variables, vp, sizeof (all_vars));
 			if (sectionname[1] == 'p')
-				section_stack.count--;
+				Variables_stack.count--;
 		} else
 			return False;
 		return True;
-- 
1.6.1.220.gaabfe



More information about the rsync mailing list