Post trial install changes, round one

A bunch of changes related to my first "real" install of pacman-git into
/usr/local and trying to use it.

* Shift some uses of free -> FREE in libalpm.
* Move stat and sanity checks of config paths into libalpm from the
  config and argument parsing in pacman.c.
* Fix issue where dbpath still was not defined early enough due to its
  requirement for being used in alpm_db_register. This should be rewritten
  so it doesn't have this dependency, but this will work for now.

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

lib/libalpm/alpm.h

@@ -93,17 +93,17 @@ alpm_cb_download alpm_option_get_dlcb();
 void alpm_option_set_dlcb(alpm_cb_download cb);
 
 const char *alpm_option_get_root();
-void alpm_option_set_root(const char *root);
+int alpm_option_set_root(const char *root);
 
 const char *alpm_option_get_dbpath();
-void alpm_option_set_dbpath(const char *dbpath);
+int alpm_option_set_dbpath(const char *dbpath);
 
 alpm_list_t *alpm_option_get_cachedirs();
-void alpm_option_add_cachedir(const char *cachedir);
+int alpm_option_add_cachedir(const char *cachedir);
 void alpm_option_set_cachedirs(alpm_list_t *cachedirs);
 
 const char *alpm_option_get_logfile();
-void alpm_option_set_logfile(const char *logfile);
+int alpm_option_set_logfile(const char *logfile);
 
 const char *alpm_option_get_lockfile();
 /* no set_lockfile, path is determined from dbpath */
@@ -145,7 +145,7 @@ alpm_list_t *alpm_option_get_syncdbs();
 
 pmdb_t *alpm_db_register(const char *treename);
 int alpm_db_unregister(pmdb_t *db);
-int alpm_db_unregister_all();
+int alpm_db_unregister_all(void);
 
 const char *alpm_db_get_name(const pmdb_t *db);
 const char *alpm_db_get_url(const pmdb_t *db);
@@ -163,7 +163,7 @@ alpm_list_t *alpm_db_getgrpcache(pmdb_t *db);
 alpm_list_t *alpm_db_test(pmdb_t *db);
 alpm_list_t *alpm_db_search(pmdb_t *db, const alpm_list_t* needles);
 
-alpm_list_t *alpm_db_get_upgrades();
+alpm_list_t *alpm_db_get_upgrades(void);
 
 /*
  * Packages
@@ -389,6 +389,7 @@ enum _pmerrno_t {
 	PM_ERR_SYSTEM,
 	PM_ERR_BADPERMS,
 	PM_ERR_NOT_A_FILE,
+	PM_ERR_NOT_A_DIR,
 	PM_ERR_WRONG_ARGS,
 	/* Interface */
 	PM_ERR_HANDLE_NULL,

lib/libalpm/conflict.c

@@ -62,7 +62,7 @@ static int does_conflict(pmpkg_t *pkg1, const char *conflict, pmpkg_t *pkg2)
 		_alpm_log(PM_LOG_DEBUG, "package %s conflicts with %s (by %s)",
 				pkg1name, pkg2name, conflict);
 	}
-	free(conf);
+	FREE(conf);
 	return(match);
 }
 

lib/libalpm/db.c

@@ -73,7 +73,7 @@ pmdb_t SYMEXPORT *alpm_db_register(const char *treename)
 /** Unregister all package databases
  * @return 0 on success, -1 on error (pm_errno is set accordingly)
  */
-int SYMEXPORT alpm_db_unregister_all()
+int SYMEXPORT alpm_db_unregister_all(void)
 {
 	alpm_list_t *i;
 
@@ -431,7 +431,7 @@ alpm_list_t SYMEXPORT *alpm_db_search(pmdb_t *db, const alpm_list_t* needles)
 /** Get a list of upgradable packages on the current system
  * @return a pmsyncpkg_t list of packages that are out of date
  */
-alpm_list_t SYMEXPORT *alpm_db_get_upgrades()
+alpm_list_t SYMEXPORT *alpm_db_get_upgrades(void)
 {
 	alpm_list_t *syncpkgs = NULL;
 	const alpm_list_t *i, *j, *k, *m;
@@ -592,15 +592,13 @@ pmdb_t *_alpm_db_new(const char *dbpath, const char *treename)
 
 	db = calloc(1, sizeof(pmdb_t));
 	if(db == NULL) {
-		_alpm_log(PM_LOG_ERROR, _("malloc failed: could not allocate %d bytes"),
+		_alpm_log(PM_LOG_ERROR, "calloc : %s\n", strerror(errno));
-				  sizeof(pmdb_t));
 		RET_ERR(PM_ERR_MEMORY, NULL);
 	}
 
 	db->path = calloc(1, pathsize);
 	if(db->path == NULL) {
-		_alpm_log(PM_LOG_ERROR, _("malloc failed: could not allocate %d bytes"),
+		_alpm_log(PM_LOG_ERROR, "calloc : %s\n", strerror(errno));
-				  pathsize);
 		FREE(db);
 		RET_ERR(PM_ERR_MEMORY, NULL);
 	}
@@ -723,6 +721,7 @@ pmdb_t *_alpm_db_register(const char *treename)
 			RET_ERR(PM_ERR_DB_OPEN, NULL);
 	}
 	snprintf(path, PATH_MAX, "%s%s", dbpath, treename);
+	/* TODO this is rediculous, we try to do this even if we can't */
 	if(stat(path, &buf) != 0 || !S_ISDIR(buf.st_mode)) {
 		_alpm_log(PM_LOG_DEBUG, "database dir '%s' does not exist, creating it",
 				path);
@@ -731,7 +730,7 @@ pmdb_t *_alpm_db_register(const char *treename)
 		}
 	}
 
-	db = _alpm_db_new(handle->dbpath, treename);
+	db = _alpm_db_new(dbpath, treename);
 	if(db == NULL) {
 		RET_ERR(PM_ERR_DB_CREATE, NULL);
 	}

lib/libalpm/deps.c

@@ -330,7 +330,7 @@ alpm_list_t *_alpm_checkdeps(pmdb_t *db, pmtranstype_t op,
 							}
 						}
 					}
-					free(depend);
+					FREE(depend);
 				}
 			}
 		}
@@ -378,7 +378,7 @@ alpm_list_t *_alpm_checkdeps(pmdb_t *db, pmtranstype_t op,
 						FREE(miss);
 					}
 				}
-				free(depend);
+				FREE(depend);
 			}
 		}
 	} else if(op == PM_TRANS_TYPE_REMOVE) {
@@ -434,7 +434,7 @@ alpm_list_t *_alpm_checkdeps(pmdb_t *db, pmtranstype_t op,
 							}
 						}
 					}
-					free(depend);
+					FREE(depend);
 				}
 			}
 		}
@@ -632,7 +632,7 @@ void _alpm_recursedeps(pmdb_t *db, alpm_list_t **targs, int include_explicit)
 						ready = 0;
 					}
 				}
-				free(depend);
+				FREE(depend);
 			}
 		}
 	}

lib/libalpm/error.c

@@ -42,6 +42,8 @@ const char SYMEXPORT *alpm_strerror(int err)
 			return _("insufficient privileges");
 		case PM_ERR_NOT_A_FILE:
 			return _("could not find or read file");
+		case PM_ERR_NOT_A_DIR:
+			return _("could not find or read directory");
 		case PM_ERR_WRONG_ARGS:
 			return _("wrong or NULL argument passed");
 		/* Interface */

lib/libalpm/handle.c

@@ -30,6 +30,8 @@
 #include <sys/types.h>
 #include <syslog.h>
 #include <time.h>
+#include <sys/stat.h>
+#include <errno.h>
 
 /* libalpm */
 #include "handle.h"
@@ -126,70 +128,103 @@ void SYMEXPORT alpm_option_set_logcb(alpm_cb_log cb) { handle->logcb = cb; }
 
 void SYMEXPORT alpm_option_set_dlcb(alpm_cb_download cb) { handle->dlcb = cb; }
 
-void SYMEXPORT alpm_option_set_root(const char *root)
+int SYMEXPORT alpm_option_set_root(const char *root)
 {
+	struct stat st;
+	char *realroot;
+	size_t rootlen;
+
 	ALPM_LOG_FUNC;
 
-	if(handle->root) FREE(handle->root);
+	if(!root) {
+		pm_errno = PM_ERR_WRONG_ARGS;
+		return(-1);
+	}
+	if(stat(root, &st) == -1 || !S_ISDIR(st.st_mode)) {
+		pm_errno = PM_ERR_NOT_A_DIR;
+		return(-1);
+	}
 	/* According to the man page, realpath is safe to use IFF the second arg is
 	 * NULL. */
-	char *realroot = realpath(root, NULL);
+	realroot = realpath(root, NULL);
-	if(realroot) {
+	if(!realroot) {
-		root = realroot;
+		pm_errno = PM_ERR_NOT_A_DIR;
-	} else {
+		return(-1);
-		_alpm_log(PM_LOG_ERROR, _("cannot canonicalize specified root path '%s'"), root);
 	}
 
-	if(root) {
 	/* verify root ends in a '/' */
-		int rootlen = strlen(realroot);
+	rootlen = strlen(realroot);
 	if(realroot[rootlen-1] != '/') {
 		rootlen += 1;
 	}
-		handle->root = calloc(rootlen+1, sizeof(char));
+	if(handle->root) {
+		FREE(handle->root);
+	}
+	handle->root = calloc(rootlen + 1, sizeof(char));
 	strncpy(handle->root, realroot, rootlen);
 	handle->root[rootlen-1] = '/';
-	}
+	FREE(realroot);
-	if(realroot) {
-		free(realroot);
-	}
 	_alpm_log(PM_LOG_DEBUG, "option 'root' = %s", handle->root);
+	return(0);
 }
 
-void SYMEXPORT alpm_option_set_dbpath(const char *dbpath)
+int SYMEXPORT alpm_option_set_dbpath(const char *dbpath)
 {
+	struct stat st;
+	size_t dbpathlen, lockfilelen;
+	const char *lf = "db.lck";
+
 	ALPM_LOG_FUNC;
 
-	if(handle->dbpath) FREE(handle->dbpath);
+	if(!dbpath) {
-	if(handle->lockfile) FREE(handle->lockfile);
+		pm_errno = PM_ERR_WRONG_ARGS;
-	if(dbpath) {
+		return(-1);
+	}
+	if(stat(dbpath, &st) == -1 || !S_ISDIR(st.st_mode)) {
+		pm_errno = PM_ERR_NOT_A_DIR;
+		return(-1);
+	}
 	/* verify dbpath ends in a '/' */
-		int dbpathlen = strlen(dbpath);
+	dbpathlen = strlen(dbpath);
 	if(dbpath[dbpathlen-1] != '/') {
 		dbpathlen += 1;
 	}
+	if(handle->dbpath) {
+		FREE(handle->dbpath);
+	}
 	handle->dbpath = calloc(dbpathlen+1, sizeof(char));
 	strncpy(handle->dbpath, dbpath, dbpathlen);
 	handle->dbpath[dbpathlen-1] = '/';
 	_alpm_log(PM_LOG_DEBUG, "option 'dbpath' = %s", handle->dbpath);
 
-		const char *lf = "db.lck";
+	if(handle->lockfile) {
-		int lockfilelen = strlen(handle->dbpath) + strlen(lf) + 1;
+		FREE(handle->lockfile);
+	}
+	lockfilelen = strlen(handle->dbpath) + strlen(lf) + 1;
 	handle->lockfile = calloc(lockfilelen, sizeof(char));
 	snprintf(handle->lockfile, lockfilelen, "%s%s", handle->dbpath, lf);
 	_alpm_log(PM_LOG_DEBUG, "option 'lockfile' = %s", handle->lockfile);
-	}
+	return(0);
-
 }
 
-void SYMEXPORT alpm_option_add_cachedir(const char *cachedir)
+int SYMEXPORT alpm_option_add_cachedir(const char *cachedir)
 {
+	struct stat st;
+	char *newcachedir;
+	size_t cachedirlen;
+
 	ALPM_LOG_FUNC;
 
-	if(cachedir) {
+	if(!cachedir) {
-		char *newcachedir;
+		pm_errno = PM_ERR_WRONG_ARGS;
+		return(-1);
+	}
+	if(stat(cachedir, &st) == -1 || !S_ISDIR(st.st_mode)) {
+		pm_errno = PM_ERR_NOT_A_DIR;
+		return(-1);
+	}
 	/* verify cachedir ends in a '/' */
-		int cachedirlen = strlen(cachedir);
+	cachedirlen = strlen(cachedir);
 	if(cachedir[cachedirlen-1] != '/') {
 		cachedirlen += 1;
 	}
@@ -198,7 +233,7 @@ void SYMEXPORT alpm_option_add_cachedir(const char *cachedir)
 	newcachedir[cachedirlen-1] = '/';
 	handle->cachedirs = alpm_list_add(handle->cachedirs, newcachedir);
 	_alpm_log(PM_LOG_DEBUG, "option 'cachedir' = %s", newcachedir);
-	}
+	return(0);
 }
 
 void SYMEXPORT alpm_option_set_cachedirs(alpm_list_t *cachedirs)
@@ -207,21 +242,48 @@ void SYMEXPORT alpm_option_set_cachedirs(alpm_list_t *cachedirs)
 	if(cachedirs) handle->cachedirs = cachedirs;
 }
 
-void SYMEXPORT alpm_option_set_logfile(const char *logfile)
+int SYMEXPORT alpm_option_set_logfile(const char *logfile)
 {
+	char *oldlogfile = handle->logfile;
+	FILE *oldlogstream = handle->logstream;
+
 	ALPM_LOG_FUNC;
 
-	if(handle->logfile) {
+	if(!logfile) {
-		FREE(handle->logfile);
+		pm_errno = PM_ERR_WRONG_ARGS;
-		if(handle->logstream) {
+		return(-1);
-			fclose(handle->logstream);
-			handle->logstream = NULL;
 	}
-	}
+
-	if(logfile) {
 	handle->logfile = strdup(logfile);
 	handle->logstream = fopen(logfile, "a");
+	if(handle->logstream == NULL) {
+		/* TODO we probably want to do this at some point, but right now
+		 * it just blows up when a user calls pacman without privilages */
+		_alpm_log(PM_LOG_DEBUG, "couldn't open logfile for writing, ignoring");
+		/*
+		if(errno == EACCES) {
+			pm_errno = PM_ERR_BADPERMS;
+		} else if(errno == ENOENT) {
+			pm_errno = PM_ERR_NOT_A_DIR;
+		} else {
+			pm_errno = PM_ERR_SYSTEM;
 		}
+		* reset logfile to its previous value *
+		FREE(handle->logfile);
+		handle->logfile = oldlogfile;
+		handle->logstream = oldlogstream;
+		return(-1);
+		*/
+	}
+
+	if(oldlogfile) {
+		FREE(oldlogfile);
+	}
+	if(oldlogstream) {
+		fclose(oldlogstream);
+	}
+	_alpm_log(PM_LOG_DEBUG, "option 'logfile' = %s", handle->logfile);
+	return(0);
 }
 
 void SYMEXPORT alpm_option_set_usesyslog(unsigned short usesyslog)
@@ -255,6 +317,7 @@ void SYMEXPORT alpm_option_add_ignorepkg(const char *pkg)
 {
 	handle->ignorepkg = alpm_list_add(handle->ignorepkg, strdup(pkg));
 }
+
 void alpm_option_set_ignorepkgs(alpm_list_t *ignorepkgs)
 {
 	if(handle->ignorepkg) FREELIST(handle->ignorepkg);
@@ -265,6 +328,7 @@ void SYMEXPORT alpm_option_add_holdpkg(const char *pkg)
 {
 	handle->holdpkg = alpm_list_add(handle->holdpkg, strdup(pkg));
 }
+
 void SYMEXPORT alpm_option_set_holdpkgs(alpm_list_t *holdpkgs)
 {
 	if(handle->holdpkg) FREELIST(handle->holdpkg);

lib/libalpm/package.c

@@ -1137,7 +1137,7 @@ void _alpm_pkg_update_requiredby(pmpkg_t *pkg)
 			}
 			
 			satisfies = alpm_depcmp(pkg, dep);
-			free(dep);
+			FREE(dep);
 			if(satisfies) {
 				alpm_list_t *reqs = alpm_pkg_get_requiredby(pkg);
 				_alpm_log(PM_LOG_DEBUG, "adding '%s' in requiredby field for '%s'",

lib/libalpm/server.c

@@ -133,7 +133,7 @@ static struct url *url_for_file(pmserver_t *server, const char *filename)
 												doc,
 												server->s_url->user,
 												server->s_url->pwd);
-	free(doc);
+	FREE(doc);
 	return(ret);
 }
 

lib/libalpm/sync.c

@@ -734,7 +734,7 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)
 						/* file is not in the cache dir, so add it to the list */
 						files = alpm_list_add(files, strdup(fname));
 					}
-					free(fpath);
+					FREE(fpath);
 				}
 			}
 		}
@@ -805,7 +805,7 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)
 			*data = alpm_list_add(*data, ptr);
 			retval = 1;
 		}
-		free(filepath);
+		FREE(filepath);
 		FREE(md5sum2);
 	}
 	if(retval) {
@@ -885,10 +885,10 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)
 		fpath = _alpm_filecache_find(fname);
 
 		if(_alpm_trans_addtarget(tr, fpath) == -1) {
-			free(fpath);
+			FREE(fpath);
 			goto error;
 		}
-		free(fpath);
+		FREE(fpath);
 
 		/* using alpm_list_last() is ok because addtarget() adds the new target at the
 		 * end of the tr->packages list */

lib/libalpm/trans.c

@@ -488,7 +488,7 @@ int _alpm_trans_update_depends(pmtrans_t *trans, pmpkg_t *pkg)
 				}
 			}
 		}
-		free(dep);
+		FREE(dep);
 	}
 	return(0);
 }

lib/libalpm/util.c

@@ -364,7 +364,7 @@ int _alpm_lckmk()
 		}
 	}
 
-	free(dir);
+	FREE(dir);
 
 	return(fd > 0 ? fd : -1);
 }

src/pacman/conf.c

@@ -23,6 +23,7 @@
 
 #include <stdlib.h>
 #include <stdio.h>
+#include <string.h> /* strdup */
 
 /* pacman */
 #include "conf.h"
@@ -34,8 +35,11 @@ config_t *config_new(void)
 			fprintf(stderr, "malloc failure: could not allocate %d bytes\n",
 			        sizeof(config_t));
 	}
+	/* defaults which may get overridden later */
 	config->op = PM_OP_MAIN;
 	config->logmask = PM_LOG_ERROR | PM_LOG_WARNING;
+	/* CONFFILE is defined at compile-time */
+	config->configfile = strdup(CONFFILE);
 
 	return(config);
 }

src/pacman/conf.h

@@ -24,7 +24,6 @@
 #include <alpm.h>
 
 typedef struct __config_t {
-	/* command line options */
 	char *configfile;
 	unsigned short op;
 	unsigned short verbose;
@@ -34,8 +33,11 @@ typedef struct __config_t {
 	unsigned short noconfirm;
 	unsigned short noprogressbar;
 	unsigned short logmask;
+	/* keep track if we had paths specified on command line */
+	unsigned short have_root;
+	unsigned short have_dbpath;
+	unsigned short have_logfile;
 
-	/* command line options */
 	unsigned short op_q_isfile;
 	unsigned short op_q_info;
 	unsigned short op_q_list;
@@ -48,6 +50,7 @@ typedef struct __config_t {
 	unsigned short op_q_changelog;
 	unsigned short op_q_test;
 	unsigned short op_q_upgrade;
+
 	unsigned short op_s_clean;
 	unsigned short op_s_dependsonly;
 	unsigned short op_s_downloadonly;
@@ -55,10 +58,12 @@ typedef struct __config_t {
 	unsigned short op_s_sync;
 	unsigned short op_s_search;
 	unsigned short op_s_upgrade;
+
 	unsigned short group;
 	pmtransflag_t flags;
 	unsigned short noask;
 	unsigned int ask;
+
 	/* conf file options */
 	unsigned short chomp; /* I Love Candy! */
 	unsigned short usecolor; /* enable colorful output */

src/pacman/pacman.c

@@ -1,7 +1,7 @@
 /*
  *  pacman.c
  *
- *  Copyright (c) 2002-2006 by Judd Vinet <jvinet@zeroflux.org>
+ *  Copyright (c) 2002-2007 by Judd Vinet <jvinet@zeroflux.org>
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -29,7 +29,6 @@
 #include <signal.h>
 #include <unistd.h>
 #include <sys/types.h>
-#include <sys/stat.h>
 #include <sys/utsname.h> /* uname */
 #include <libintl.h> /* bindtextdomain, textdomain */
 #include <locale.h> /* setlocale */
@@ -288,7 +287,6 @@ static int parseargs(int argc, char *argv[])
 		{"test",       no_argument,       0, 1009},
 		{0, 0, 0, 0}
 	};
-	struct stat st;
 
 	while((opt = getopt_long(argc, argv, "ARUFQSTr:b:vkhscVfmnoldepituwygz", opts, &option_index))) {
 		if(opt < 0) {
@@ -331,13 +329,11 @@ static int parseargs(int argc, char *argv[])
 			case 1005: config->flags |= PM_TRANS_FLAG_NOSCRIPTLET; break;
 			case 1006: config->noask = 1; config->ask = atoi(optarg); break;
 			case 1007:
-				/* TODO redo this logic- check path somewhere else, delete other cachedirs, etc */
+				if(alpm_option_add_cachedir(optarg) != 0) {
-				if(stat(optarg, &st) == -1 || !S_ISDIR(st.st_mode)) {
+					pm_printf(PM_LOG_ERROR, _("problem adding cachedir '%s' (%s)\n"),
-					pm_printf(PM_LOG_ERROR, _("'%s' is not a valid cache directory\n"),
+							optarg, alpm_strerror(pm_errno));
-							optarg);
 					return(1);
 				}
-				alpm_option_add_cachedir(optarg);
 				break;
 			case 1008:
 				config->flags |= PM_TRANS_FLAG_ALLDEPS;
@@ -357,12 +353,12 @@ static int parseargs(int argc, char *argv[])
 			case 'U': config->op = (config->op != PM_OP_MAIN ? 0 : PM_OP_UPGRADE); break;
 			case 'V': config->version = 1; break;
 			case 'b':
-				if(stat(optarg, &st) == -1 || !S_ISDIR(st.st_mode)) {
+				if(alpm_option_set_dbpath(optarg) != 0) {
-					pm_printf(PM_LOG_ERROR, _("'%s' is not a valid db path\n"),
+					pm_printf(PM_LOG_ERROR, _("problem setting dbpath '%s' (%s)\n"),
-							optarg);
+							optarg, alpm_strerror(pm_errno));
 					return(1);
 				}
-				alpm_option_set_dbpath(optarg);
+				config->have_dbpath = 1;
 				break;
 			case 'c':
 				(config->op_s_clean)++;
@@ -391,12 +387,12 @@ static int parseargs(int argc, char *argv[])
 				config->flags |= PM_TRANS_FLAG_PRINTURIS;
 				break;
 			case 'r':
-				if(stat(optarg, &st) == -1 || !S_ISDIR(st.st_mode)) {
+				if(alpm_option_set_root(optarg) != 0) {
-					pm_printf(PM_LOG_ERROR, _("'%s' is not a valid root path\n"),
+					pm_printf(PM_LOG_ERROR, _("problem setting root '%s' (%s)\n"),
-							optarg);
+							optarg, alpm_strerror(pm_errno));
 					return(1);
 				}
-				alpm_option_set_root(optarg);
+				config->have_root = 1;
 				break;
 			case 's':
 				config->op_s_search = 1;
@@ -455,7 +451,6 @@ static int _parseconfig(const char *file, const char *givensection,
 	int linenum = 0;
 	char *ptr, *section = NULL;
 	pmdb_t *db = NULL;
-	struct stat st;
 
 	pm_printf(PM_LOG_DEBUG, "config: attempting to read file %s\n", file);
 	fp = fopen(file, "r");
@@ -619,37 +614,38 @@ static int _parseconfig(const char *file, const char *givensection,
 						pm_printf(PM_LOG_DEBUG, "config: holdpkg: %s\n", p);
 					} else if(strcmp(key, "DBPath") == 0 || strcmp(upperkey, "DBPATH") == 0) {
 						/* don't overwrite a path specified on the command line */
-						if(alpm_option_get_dbpath() == NULL) {
+						if(!config->have_dbpath) {
-							if(stat(ptr, &st) == -1 || !S_ISDIR(st.st_mode)) {
+							if(alpm_option_set_dbpath(ptr) != 0) {
-								pm_printf(PM_LOG_ERROR, _("'%s' is not a valid db path\n"),
+								pm_printf(PM_LOG_ERROR, _("problem setting dbpath '%s' (%s)\n"),
-										ptr);
+										ptr, alpm_strerror(pm_errno));
 								return(1);
 							}
-							alpm_option_set_dbpath(ptr);
 							pm_printf(PM_LOG_DEBUG, "config: dbpath: %s\n", ptr);
 						}
 					} else if(strcmp(key, "CacheDir") == 0 || strcmp(upperkey, "CACHEDIR") == 0) {
-						if(stat(ptr, &st) == -1 || !S_ISDIR(st.st_mode)) {
+						if(alpm_option_add_cachedir(ptr) != 0) {
-							pm_printf(PM_LOG_WARNING, _("'%s' is not a valid cache directory\n"),
+							pm_printf(PM_LOG_ERROR, _("problem adding cachedir '%s' (%s)\n"),
-									ptr);
+									ptr, alpm_strerror(pm_errno));
-						} else {
+							return(1);
-							alpm_option_add_cachedir(ptr);
+						}
-							pm_printf(PM_LOG_DEBUG, "config: cachedir: %s\n", ptr);
+						pm_printf(PM_LOG_DEBUG, "config: cachedir: %s\n", ptr);
-						}
+					} else if(strcmp(key, "RootDir") == 0 || strcmp(upperkey, "ROOTDIR") == 0) {
-					} else if(strcmp(key, "RootDir") == 0 || strcmp(upperkey, "ROOTDIR") == 0) {
+						/* don't overwrite a path specified on the command line */
-						/* don't overwrite a path specified on the command line */
+						if(!config->have_root) {
-						if(alpm_option_get_root() == NULL) {
+							if(alpm_option_set_root(ptr) != 0) {
-							if(stat(ptr, &st) == -1 || !S_ISDIR(st.st_mode)) {
+								pm_printf(PM_LOG_ERROR, _("problem setting root '%s' (%s)\n"),
-								pm_printf(PM_LOG_ERROR, _("'%s' is not a valid root path\n"),
+										ptr, alpm_strerror(pm_errno));
-										ptr);
 								return(1);
 							}
-							alpm_option_set_root(ptr);
 							pm_printf(PM_LOG_DEBUG, "config: rootdir: %s\n", ptr);
 						}
 					} else if (strcmp(key, "LogFile") == 0 || strcmp(upperkey, "LOGFILE") == 0) {
-						if(alpm_option_get_logfile() == NULL) {
+						if(!config->have_logfile) {
-							alpm_option_set_logfile(ptr);
+							if(alpm_option_set_logfile(ptr) != 0) {
+								pm_printf(PM_LOG_ERROR, _("problem setting logfile '%s' (%s)\n"),
+										ptr, alpm_strerror(pm_errno));
+								return(1);
+							}
 							pm_printf(PM_LOG_DEBUG, "config: logfile: %s\n", ptr);
 						}
 					} else if (strcmp(key, "XferCommand") == 0 || strcmp(upperkey, "XFERCOMMAND") == 0) {
@@ -746,6 +742,18 @@ int main(int argc, char *argv[])
 	/* Setup logging as soon as possible, to print out maximum debugging info */
 	alpm_option_set_logcb(cb_log);
 	alpm_option_set_dlcb(cb_dl_progress);
+	/* define root and dbpath to reasonable defaults */
+	alpm_option_set_root(ROOTDIR);
+	alpm_option_set_dbpath(DBPATH);
+
+	/* Priority of options:
+	 * 1. command line
+	 * 2. config file
+	 * 3. compiled-in defaults
+	 * However, we have to parse the command line first because a config file
+	 * location can be specified here, so we need to make sure we prefer these
+	 * options over the config file coming second.
+	 */
 
 	/* parse the command line */
 	ret = parseargs(argc, argv);
@@ -753,28 +761,14 @@ int main(int argc, char *argv[])
 		cleanup(ret);
 	}
 
-	/* use default config file if location wasn't specified on cmdline */
-	if(config->configfile == NULL) {
-		config->configfile = strdup(CONFFILE);
-	}
-
 	/* parse the config file */
 	ret = parseconfig(config->configfile);
 	if(ret != 0) {
 		cleanup(ret);
 	}
 
-	/* ensure root and dbpath were defined */
-	if(alpm_option_get_root() == NULL) {
-		alpm_option_set_root(ROOTDIR);
-	}
-	if(alpm_option_get_dbpath() == NULL) {
-		alpm_option_set_dbpath(DBPATH);
-	}
-
 #if defined(HAVE_GETEUID)
 	/* check if we have sufficient permission for the requested operation */
-if(0) {
 	if(myuid > 0) {
 		if(config->op != PM_OP_MAIN && config->op != PM_OP_QUERY && config->op != PM_OP_DEPTEST) {
 			if((config->op == PM_OP_SYNC && !config->op_s_sync &&
@@ -791,7 +785,6 @@ if(0) {
 			}
 		}
 	}
-}
 #endif
 
 	if(config->verbose > 0) {