Make config parsing a two-part affair

This ensures we call any alpm_option type functions before registering
databases, making sure all paths and other defaults (e.g. sig
verification levels) have been set first. This will ensure we can
continue to allow crazy config files where [options] doesn't come first.

The diffstat on this commit is misleading; view with
-w/--ignore-all-space to get a better idea of what needed to be touched.

Signed-off-by: Dan McGee <dan@archlinux.org>

src/pacman/conf.c

@@ -405,10 +405,8 @@ static int _add_mirror(pmdb_t *db, char *value)
  * of our paths to live under the rootdir that was specified. Safe to call
  * multiple times (will only do anything the first time).
  */
-static void setlibpaths(void)
+static int setlibpaths(void)
 {
-	static int init = 0;
-	if(!init) {
 	int ret = 0;
 
 	pm_printf(PM_LOG_DEBUG, "setlibpaths() called\n");
@@ -420,7 +418,7 @@ static void setlibpaths(void)
 		if(ret != 0) {
 			pm_printf(PM_LOG_ERROR, _("problem setting rootdir '%s' (%s)\n"),
 					config->rootdir, alpm_strerrorlast());
-				cleanup(ret);
+			return ret;
 		}
 		if(!config->dbpath) {
 			/* omit leading slash from our static DBPATH, root handles it */
@@ -441,7 +439,7 @@ static void setlibpaths(void)
 		if(ret != 0) {
 			pm_printf(PM_LOG_ERROR, _("problem setting dbpath '%s' (%s)\n"),
 					config->dbpath, alpm_strerrorlast());
-				cleanup(ret);
+			return ret;
 		}
 	}
 	if(config->logfile) {
@@ -449,7 +447,7 @@ static void setlibpaths(void)
 		if(ret != 0) {
 			pm_printf(PM_LOG_ERROR, _("problem setting logfile '%s' (%s)\n"),
 					config->logfile, alpm_strerrorlast());
-				cleanup(ret);
+			return ret;
 		}
 	}
 
@@ -460,7 +458,7 @@ static void setlibpaths(void)
 		if(ret != 0) {
 			pm_printf(PM_LOG_ERROR, _("problem setting gpgdir '%s' (%s)\n"),
 					config->gpgdir, alpm_strerrorlast());
-				cleanup(ret);
+			return ret;
 		}
 	}
 
@@ -468,15 +466,14 @@ static void setlibpaths(void)
 	if(alpm_option_get_cachedirs() == NULL) {
 		alpm_option_add_cachedir(CACHEDIR);
 	}
-		init = 1;
+	return 0;
-	}
 }
 
 
 /* The real parseconfig. Called with a null section argument by the publicly
  * visible parseconfig so we can recall from within ourself on an include */
-static int _parseconfig(const char *file, const char *givensection,
+static int _parseconfig(const char *file, int parse_options,
-                        pmdb_t * const givendb)
+		const char *givensection, pmdb_t * const givendb)
 {
 	FILE *fp = NULL;
 	char line[PATH_MAX+1];
@@ -502,6 +499,8 @@ static int _parseconfig(const char *file, const char *givensection,
 	}
 
 	while(fgets(line, PATH_MAX, fp)) {
+		char *key, *value;
+
 		linenum++;
 		strtrim(line);
 
@@ -513,6 +512,14 @@ static int _parseconfig(const char *file, const char *givensection,
 			*ptr = '\0';
 		}
 
+		/* sanity check */
+		if(parse_options && db) {
+			pm_printf(PM_LOG_ERROR, _("config file %s, line %d: parsing options but have a database.\n"),
+						file, linenum);
+			ret = 1;
+			goto cleanup;
+		}
+
 		if(line[0] == '[' && line[strlen(line)-1] == ']') {
 			/* new config section, skip the '[' */
 			ptr = line;
@@ -530,7 +537,7 @@ static int _parseconfig(const char *file, const char *givensection,
 				goto cleanup;
 			}
 			/* if we are not looking at the options section, register a db */
-			if(strcmp(section, "options") != 0) {
+			if(!parse_options && strcmp(section, "options") != 0) {
 				db = alpm_db_register_sync(section);
 				if(db == NULL) {
 					pm_printf(PM_LOG_ERROR, _("could not register '%s' database (%s)\n"),
@@ -543,7 +550,6 @@ static int _parseconfig(const char *file, const char *givensection,
 		}
 
 		/* directive */
-		char *key, *value;
 		/* strsep modifies the 'line' string: 'key \0 value' */
 		key = line;
 		value = line;
@@ -566,6 +572,10 @@ static int _parseconfig(const char *file, const char *givensection,
 		}
 		/* Include is allowed in both options and repo sections */
 		if(strcmp(key, "Include") == 0) {
+			glob_t globbuf;
+			int globret;
+			size_t gindex;
+
 			if(value == NULL) {
 				pm_printf(PM_LOG_ERROR, _("config file %s, line %d: directive '%s' needs a value\n"),
 						file, linenum, key);
@@ -573,8 +583,6 @@ static int _parseconfig(const char *file, const char *givensection,
 				goto cleanup;
 			}
 			/* Ignore include failures... assume non-critical */
-			int globret;
-			glob_t globbuf;
 			globret = glob(value, GLOB_NOCHECK, NULL, &globbuf);
 			switch(globret) {
 				case GLOB_NOSPACE:
@@ -593,22 +601,22 @@ static int _parseconfig(const char *file, const char *givensection,
 							file, linenum, value);
 				break;
 				default:
-					for(size_t gindex = 0; gindex < globbuf.gl_pathc; gindex++) {
+					for(gindex = 0; gindex < globbuf.gl_pathc; gindex++) {
 						pm_printf(PM_LOG_DEBUG, "config file %s, line %d: including %s\n",
 								file, linenum, globbuf.gl_pathv[gindex]);
-						_parseconfig(globbuf.gl_pathv[gindex], section, db);
+						_parseconfig(globbuf.gl_pathv[gindex], parse_options, section, db);
 					}
 				break;
 			}
 			globfree(&globbuf);
 			continue;
 		}
-		if(strcmp(section, "options") == 0) {
+		if(parse_options && strcmp(section, "options") == 0) {
 			/* we are either in options ... */
 			if((ret = _parse_options(key, value, file, linenum)) != 0) {
 				goto cleanup;
 			}
-		} else {
+		} else if (!parse_options && strcmp(section, "options") != 0) {
 			/* ... or in a repo section */
 			if(strcmp(key, "Server") == 0) {
 				if(value == NULL) {
@@ -645,7 +653,6 @@ static int _parseconfig(const char *file, const char *givensection,
 						file, linenum, key, section);
 			}
 		}
-
 	}
 
 cleanup:
@@ -653,8 +660,6 @@ cleanup:
 	if(section){
 		free(section);
 	}
-	/* call setlibpaths here to ensure we have called it at least once */
-	setlibpaths();
 	pm_printf(PM_LOG_DEBUG, "config: finished parsing %s\n", file);
 	return ret;
 }
@@ -665,8 +670,23 @@ cleanup:
  */
 int parseconfig(const char *file)
 {
+	int ret;
+	/* the config parse is a two-pass affair. We first parse the entire thing for
+	 * the [options] section so we can get all default and path options set.
+	 * Next, we go back and parse everything but [options]. */
+
 	/* call the real parseconfig function with a null section & db argument */
-	return _parseconfig(file, NULL, NULL);
+	pm_printf(PM_LOG_DEBUG, "parseconfig: options pass\n");
+	if((ret = _parseconfig(file, 1, NULL, NULL))) {
+		return ret;
+	}
+	/* call setlibpaths here to ensure we have called it at least once */
+	if((ret = setlibpaths())) {
+		return ret;
+	}
+	/* second pass, repo section parsing */
+	pm_printf(PM_LOG_DEBUG, "parseconfig: repo pass\n");
+	return _parseconfig(file, 0, NULL, NULL);
 }
 
 /* vim: set ts=2 sw=2 noet: */

src/pacman/pacman.c

@@ -262,7 +262,7 @@ static void setuseragent(void)
  *
  * @param ret the return value
  */
-void cleanup(int ret) {
+static void cleanup(int ret) {
 	/* free alpm library resources */
 	if(alpm_release() == -1) {
 		pm_printf(PM_LOG_ERROR, "%s\n", alpm_strerrorlast());

src/pacman/pacman.h

@@ -22,8 +22,6 @@
 
 #include <alpm_list.h>
 
-void cleanup(int ret);
-
 /* database.c */
 int pacman_database(alpm_list_t *targets);
 /* deptest.c */