* Fixed some missing error values in strerror

* Cleanup up some error enum values
* Revamped the 'pmserver_t' functionality.  Less allocation,
  removed a param and cleaned up some duplicate URL parsing

4 files changed, 108 insertions, 83 deletions

diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
index a5b5e1a4..231112f3 100644
--- a/lib/libalpm/alpm.h
+++ b/lib/libalpm/alpm.h
@@ -426,8 +426,7 @@ enum _pmerrno_t {
 	PM_ERR_DB_WRITE,
 	PM_ERR_DB_REMOVE,
 	/* Servers */
-	PM_ERR_SERVER_BAD_LOCATION,
-	PM_ERR_SERVER_PROTOCOL_UNSUPPORTED,
+	PM_ERR_SERVER_BAD_URL,
 	/* Configuration */
 	PM_ERR_OPT_LOGFILE,
 	PM_ERR_OPT_DBPATH,
@@ -443,6 +442,7 @@ enum _pmerrno_t {
 	PM_ERR_TRANS_ABORT,
 	PM_ERR_TRANS_TYPE,
 	PM_ERR_TRANS_COMMITING,
+	PM_ERR_TRANS_DOWNLOADING,
 	/* Packages */
 	PM_ERR_PKG_NOT_FOUND,
 	PM_ERR_PKG_INVALID,
@@ -474,7 +474,6 @@ enum _pmerrno_t {
 	PM_ERR_CONF_BAD_SYNTAX,
 	PM_ERR_CONF_DIRECTIVE_OUTSIDE_SECTION,
 	PM_ERR_INVALID_REGEX,
-	PM_ERR_TRANS_DOWNLOADING,
   /* Downloading */
 	PM_ERR_CONNECT_FAILED,
   PM_ERR_FORK_FAILED
diff --git a/lib/libalpm/error.c b/lib/libalpm/error.c
index a6d20acf..8befd560 100644
--- a/lib/libalpm/error.c
+++ b/lib/libalpm/error.c
@@ -36,10 +36,10 @@ char SYMEXPORT *alpm_strerror(int err)
 			return _("unexpected system error");
 		case PM_ERR_BADPERMS:
 			return _("insufficient privileges");
-		case PM_ERR_WRONG_ARGS:
-			return _("wrong or NULL argument passed");
 		case PM_ERR_NOT_A_FILE:
 			return _("could not find or read file");
+		case PM_ERR_WRONG_ARGS:
+			return _("wrong or NULL argument passed");
 		/* Interface */
 		case PM_ERR_HANDLE_NULL:
 			return _("library not initialized");
@@ -62,6 +62,9 @@ char SYMEXPORT *alpm_strerror(int err)
 			return _("could not update database");
 		case PM_ERR_DB_REMOVE:
 			return _("could not remove database entry");
+		/* Servers */
+		case PM_ERR_SERVER_BAD_URL:
+			return _("invalid url for server");
 		/* Configuration */
 		case PM_ERR_OPT_LOGFILE:
 		case PM_ERR_OPT_DBPATH:
@@ -70,10 +73,10 @@ char SYMEXPORT *alpm_strerror(int err)
 		case PM_ERR_OPT_USESYSLOG:
 			return _("could not set parameter");
 		/* Transactions */
-		case PM_ERR_TRANS_NULL:
-			return _("transaction not initialized");
 		case PM_ERR_TRANS_NOT_NULL:
 			return _("transaction already initialized");
+		case PM_ERR_TRANS_NULL:
+			return _("transaction not initialized");
 		case PM_ERR_TRANS_DUP_TARGET:
 			return _("duplicate target");
 		case PM_ERR_TRANS_NOT_INITIALIZED:
@@ -84,6 +87,10 @@ char SYMEXPORT *alpm_strerror(int err)
 			return _("transaction aborted");
 		case PM_ERR_TRANS_TYPE:
 			return _("operation not compatible with the transaction type");
+		case PM_ERR_TRANS_COMMITING:
+			return _("could not commit transaction");
+		case PM_ERR_TRANS_DOWNLOADING:
+			return _("could not download all files");
 		/* Packages */
 		case PM_ERR_PKG_NOT_FOUND:
 			return _("could not find or read package");
@@ -117,30 +124,31 @@ char SYMEXPORT *alpm_strerror(int err)
 			return _("conflicting files");
 		/* Miscellaenous */
 		case PM_ERR_USER_ABORT:
-			return _("user aborted");
-		case PM_ERR_LIBARCHIVE_ERROR:
-			return _("libarchive error");
+			return _("user aborted the operation");
 		case PM_ERR_INTERNAL_ERROR:
 			return _("internal error");
+		case PM_ERR_LIBARCHIVE_ERROR:
+			return _("libarchive error");
 		case PM_ERR_DISK_FULL:
-			return _("not enough space");
+			return _("not enough space on disk");
 		case PM_ERR_PKG_HOLD:
+			/* TODO wow this is not descriptive at all... what does this mean? */
 			return _("not confirmed");
-		/* Config */
+		/* Configuration file */
 		case PM_ERR_CONF_BAD_SECTION:
-			return _("bad section name");
+			return _("bad configuration section name");
 		case PM_ERR_CONF_LOCAL:
-			return _("'local' is reserved and cannot be used as a package tree");
+			return _("'local' is reserved and cannot be used as a repository name");
 		case PM_ERR_CONF_BAD_SYNTAX:
-			return _("syntax error");
+			return _("syntax error in config file");
 		case PM_ERR_CONF_DIRECTIVE_OUTSIDE_SECTION:
 			return _("all directives must belong to a section");
 		case PM_ERR_INVALID_REGEX:
 			return _("invalid regular expression");
+		/* Downloading */
 		case PM_ERR_CONNECT_FAILED:
 			return _("connection to remote host failed");
-		case PM_ERR_FORK_FAILED:
-			return _("forking process failed");
+		/* Unknown error! */
 		default:
 			return _("unexpected error");
 	}
diff --git a/lib/libalpm/server.c b/lib/libalpm/server.c
index 947e5860..e9713ce3 100644
--- a/lib/libalpm/server.c
+++ b/lib/libalpm/server.c
@@ -55,7 +55,7 @@ pmserver_t *_alpm_server_new(const char *url)
 	u = downloadParseURL(url);
 	if(!u) {
 		_alpm_log(PM_LOG_ERROR, _("url '%s' is invalid, ignoring"), url);
-		return(NULL);
+		RET_ERR(PM_ERR_SERVER_BAD_URL, NULL);
 	}
 	if(strlen(u->scheme) == 0) {
 		_alpm_log(PM_LOG_WARNING, _("url scheme not specified, assuming http"));
@@ -67,11 +67,7 @@ pmserver_t *_alpm_server_new(const char *url)
 		strcpy(u->pwd, "libalpm@guest");
 	}
 
-	/* This isn't needed... we can actually kill the whole pmserver_t interface
-	 * and replace it with libdownload's 'struct url'
-	 */
   server->s_url = u;
-	server->path = strdup(u->doc);
 
 	return server;
 }
@@ -87,11 +83,54 @@ void _alpm_server_free(void *data)
 	}
 
 	/* free memory */
-	FREE(server->path);
 	downloadFreeURL(server->s_url);
 	FREE(server);
 }
 
+/* remove filename info from "s_url->doc" and return it */
+static char *strip_filename(pmserver_t *server)
+{
+	char *p = NULL, *fname = NULL;
+	if(!server) {
+		return(NULL);
+	}
+
+	p = strrchr(server->s_url->doc, '/');
+	if(p && *(++p)) {
+		fname = strdup(p);
+		_alpm_log(PM_LOG_DEBUG, _("stripping '%s' from '%s'"), fname, server->s_url->doc);
+		*p = 0;
+	}
+
+	/* s_url->doc now contains ONLY path information.  return value
+	 * if the file information from the original URL */
+	return(fname);
+}
+
+/* Return a 'struct url' for this server, for downloading 'filename'. */
+static struct url *url_for_file(pmserver_t *server, const char *filename)
+{
+	struct url *ret = NULL;
+	char *doc = NULL;
+	int doclen = 0;
+
+	doclen = strlen(server->s_url->doc) + strlen(filename) + 2;
+	doc = calloc(doclen, sizeof(char));
+	if(!doc) {
+		RET_ERR(PM_ERR_MEMORY, NULL);
+	}
+
+	snprintf(doc, doclen, "%s/%s", server->s_url->doc, filename);
+	ret = downloadMakeURL(server->s_url->scheme,
+												server->s_url->host,
+												server->s_url->port,
+												doc,
+												server->s_url->user,
+												server->s_url->pwd);
+	free(doc);
+	return(ret);
+}
+
 /*
  * Download a list of files from a list of servers
  *   - if one server fails, we try the next one in the list
@@ -133,14 +172,20 @@ int _alpm_downloadfiles_forreal(alpm_list_t *servers, const char *localpath,
 	}
 
 	for(i = servers; i && !done; i = i->next) {
-		pmserver_t *server = (pmserver_t*)i->data;
+		pmserver_t *server = i->data;
 
 		/* get each file in the list */
 		for(lp = files; lp; lp = lp->next) {
+			struct url *fileurl = NULL;
 			char realfile[PATH_MAX];
 			char output[PATH_MAX];
 			char *fn = (char *)lp->data;
 
+			fileurl = url_for_file(server, fn);
+			if(!fileurl) {
+				return(-1);
+			}
+
 			snprintf(realfile, PATH_MAX, "%s/%s", localpath, fn);
 			snprintf(output, PATH_MAX, "%s/%s.part", localpath, fn);
 
@@ -156,20 +201,15 @@ int _alpm_downloadfiles_forreal(alpm_list_t *servers, const char *localpath,
 
 				if(stat(output, &st) == 0 && st.st_size > 0) {
 					_alpm_log(PM_LOG_DEBUG, _("existing file found, using it"));
-					server->s_url->offset = (off_t)st.st_size;
+					fileurl->offset = (off_t)st.st_size;
 					dltotal_bytes = st.st_size;
 					localf = fopen(output, "a");
 					chk_resume = 1;
 				} else {
-					server->s_url->offset = (off_t)0;
+					fileurl->offset = (off_t)0;
 					dltotal_bytes = 0;
 				}
 				
-				FREE(server->s_url->doc);
-				int len = strlen(server->path) + strlen(fn) + 2;
-				server->s_url->doc = (char *)malloc(len);
-				snprintf(server->s_url->doc, len, "%s/%s", server->path, fn);
-
 				/* libdownload does not reset the error code, reset it in the case of previous errors */
 				downloadLastErrCode = 0;
 
@@ -181,21 +221,21 @@ int _alpm_downloadfiles_forreal(alpm_list_t *servers, const char *localpath,
 						downloadDebug = 1;
 				}
 				if(alpm_option_get_logmask() & PM_LOG_DEBUG) {
-						dlf = downloadXGet(server->s_url, &ust, (handle->nopassiveftp ? "v" : "vp"));
+						dlf = downloadXGet(fileurl, &ust, (handle->nopassiveftp ? "v" : "vp"));
 				} else {
-						dlf = downloadXGet(server->s_url, &ust, (handle->nopassiveftp ? "" : "p"));
+						dlf = downloadXGet(fileurl, &ust, (handle->nopassiveftp ? "" : "p"));
 				}
 
 				if(downloadLastErrCode != 0 || dlf == NULL) {
-					_alpm_log(PM_LOG_ERROR, _("failed retrieving file '%s' from %s://%s : %s"), fn,
-										server->s_url->scheme, server->s_url->host, downloadLastErrString);
+					_alpm_log(PM_LOG_ERROR, _("failed retrieving file '%s' from %s : %s"),
+										fn, fileurl->host, downloadLastErrString);
 					if(localf != NULL) {
 						fclose(localf);
 					}
 					/* try the next server */
 					continue;
 				} else {
-						_alpm_log(PM_LOG_DEBUG, _("server connection to %s complete"), server->s_url->host);
+						_alpm_log(PM_LOG_DEBUG, _("connected to %s successfully"), fileurl->host);
 				}
 				
 				if(ust.mtime && mtime1) {
@@ -218,7 +258,7 @@ int _alpm_downloadfiles_forreal(alpm_list_t *servers, const char *localpath,
 					_alpm_time2string(ust.mtime, mtime2);
 				}
 
-				if(chk_resume && server->s_url->offset == 0) {
+				if(chk_resume && fileurl->offset == 0) {
 					_alpm_log(PM_LOG_WARNING, _("cannot resume download, starting over"));
 					if(localf != NULL) {
 						fclose(localf);
@@ -228,7 +268,7 @@ int _alpm_downloadfiles_forreal(alpm_list_t *servers, const char *localpath,
 
 				if(localf == NULL) {
 					_alpm_rmrf(output);
-					server->s_url->offset = (off_t)0;
+					fileurl->offset = (off_t)0;
 					dltotal_bytes = 0;
 					localf = fopen(output, "w");
 					if(localf == NULL) { /* still null? */
@@ -263,8 +303,7 @@ int _alpm_downloadfiles_forreal(alpm_list_t *servers, const char *localpath,
 				char url[PATH_MAX];
 				char cwd[PATH_MAX];
 				/* build the full download url */
-				snprintf(url, PATH_MAX, "%s://%s%s/%s", server->s_url->scheme, server->s_url->host,
-						server->s_url->doc, fn);
+				snprintf(url, PATH_MAX, "%s://%s%s/%s", fileurl->scheme, fileurl->host, fileurl->doc, fn);
 				/* replace all occurrences of %o with fn.part */
 				strncpy(origCmd, handle->xfercommand, sizeof(origCmd));
 				ptr1 = origCmd;
@@ -315,6 +354,7 @@ int _alpm_downloadfiles_forreal(alpm_list_t *servers, const char *localpath,
 				}
 				chdir(cwd);
 			}
+			downloadFreeURL(fileurl);
 		}
 
 		if(alpm_list_count(complete) == alpm_list_count(files)) {
@@ -327,66 +367,44 @@ int _alpm_downloadfiles_forreal(alpm_list_t *servers, const char *localpath,
 
 char *_alpm_fetch_pkgurl(char *target)
 {
-	char *p = NULL;
+	pmserver_t *server;
+	char *filename;
 	struct stat st;
-	struct url *s_url;
 
 	ALPM_LOG_FUNC;
 
-	s_url = downloadParseURL(target);
-	if(!s_url) {
-		_alpm_log(PM_LOG_ERROR, _("url '%s' is invalid, ignoring"), target);
+	server = _alpm_server_new(target);
+	if(!server) {
 		return(NULL);
 	}
-	if(strlen(s_url->scheme) == 0) {
-		_alpm_log(PM_LOG_WARNING, _("url scheme not specified, assuming http"));
-		strcpy(s_url->scheme, "http");
-	}
 
-	if(strcmp(s_url->scheme,"ftp") == 0 && strlen(s_url->user) == 0) {
-		strcpy(s_url->user, "anonymous");
-		strcpy(s_url->pwd, "libalpm@guest");
+	/* strip path information from the filename */
+	filename = strip_filename(server);
+	if(!filename) {
+		_alpm_log(PM_LOG_ERROR, _("URL does not contain a file for download"));
+		return(NULL);
 	}
 
 	/* do not download the file if it exists in the current dir */
-	if(stat(s_url->doc, &st) == 0) {
-		_alpm_log(PM_LOG_DEBUG, _(" %s is already in the current directory"), s_url->doc);
+	if(stat(filename, &st) == 0) {
+		_alpm_log(PM_LOG_DEBUG, _("%s has already been downloaded"), filename);
 	} else {
-		pmserver_t *server;
-		alpm_list_t *servers = NULL;
-		alpm_list_t *files;
+		alpm_list_t *servers = alpm_list_add(NULL, server);
+		alpm_list_t *files = alpm_list_add(NULL, filename);
 
-		if((server = (pmserver_t *)malloc(sizeof(pmserver_t))) == NULL) {
-			_alpm_log(PM_LOG_ERROR, _("malloc failure: could not allocate %d bytes"), sizeof(pmserver_t));
+		if(_alpm_downloadfiles(servers, ".", files)) {
+			_alpm_log(PM_LOG_WARNING, _("failed to download %s"), target);
 			return(NULL);
 		}
-		if(s_url->doc && (p = strrchr(s_url->doc,'/'))) {
-			*p++ = '\0';
-			_alpm_log(PM_LOG_DEBUG, _("downloading '%s' from '%s://%s%s"), p, s_url->scheme, s_url->host, s_url->doc);
-
-			server->s_url = s_url;
-			server->path = strdup(s_url->doc);
-			servers = alpm_list_add(servers, server);
-
-			files = alpm_list_add(NULL, strdup(p));
-			if(_alpm_downloadfiles(servers, ".", files)) {
-				_alpm_log(PM_LOG_WARNING, _("failed to download %s"), target);
-				return(NULL);
-			}
-			FREELISTPTR(files);
-			FREELIST(servers);
-		}
-	}
-
-	/* dupe before we free the URL struct...*/
-	if(p) {
-		p = strdup(p);
+		_alpm_log(PM_LOG_DEBUG, _("successfully downloaded %s"), filename);
+		alpm_list_free(files);
+		alpm_list_free(servers);
 	}
 
-	downloadFreeURL(s_url);
+	_alpm_server_free(server);
 
 	/* return the target with the raw filename, no URL */
-	return(p);
+	return(filename);
 }
 
 /* vim: set ts=2 sw=2 noet: */
diff --git a/lib/libalpm/server.h b/lib/libalpm/server.h
index 63dd62ea..60c9163c 100644
--- a/lib/libalpm/server.h
+++ b/lib/libalpm/server.h
@@ -32,7 +32,7 @@
 
 /* Servers */
 struct __pmserver_t {
-	char *path;
+	/* useless abstraction now? */
 	struct url *s_url;
 };