Refactoring of the download code.

This should be the main step in the download refactoring initiated by commit
81a2a06818.
The stub functions introduced by that commit were implemented.

The big download code was mostly composed of two steps, and so it has been
naturally splitted in two functions :  download_external and download_internal

file:/// urls are now handled manually, instead of forcing the use of the
internal downloader.

Thanks to Dan for fixing the remaining issues and cleaning up the patch :)

Signed-off-by: Chantry Xavier <shiningxc@gmail.com>

lib/libalpm/dload.c

@@ -35,22 +35,13 @@
 #include "error.h"
 #include "handle.h"
 
-/* Return a 'struct url' for this server, for downloading 'filename'. */
+/* Build a 'struct url' from an url. */
-static struct url *url_for_file(const char *url, const char *filename)
+static struct url *url_for_string(const char *url)
 {
 	struct url *ret = NULL;
-	char *buf = NULL;
+	ret = downloadParseURL(url);
-	int len;
-
-	/* print url + filename into a buffer */
-	len = strlen(url) + strlen(filename) + 2;
-	CALLOC(buf, len, sizeof(char), RET_ERR(PM_ERR_MEMORY, NULL));
-	snprintf(buf, len, "%s/%s", url, filename);
-
-	ret = downloadParseURL(buf);
-	FREE(buf);
 	if(!ret) {
-		_alpm_log(PM_LOG_ERROR, _("url '%s' is invalid\n"), buf);
+		_alpm_log(PM_LOG_ERROR, _("url '%s' is invalid\n"), url);
 		RET_ERR(PM_ERR_SERVER_BAD_URL, NULL);
 	}
 
@@ -68,109 +59,64 @@ static struct url *url_for_file(const char *url, const char *filename)
 	return(ret);
 }
 
-/* TODO temporary private declaration */
+static char *get_filename(const char *url) {
-int _alpm_downloadfiles_forreal(alpm_list_t *servers, const char *localpath,
+	char *filename = strrchr(url, '/');
-	alpm_list_t *files, time_t mtime1, time_t *mtime2);
+	if(filename != NULL) {
-
+		filename++;
-
+	}
-/* TODO implement these as real functions */
+	return(filename);
-int _alpm_download_single_file(const char *filename,
-		alpm_list_t *servers, const char *localpath,
-		time_t mtimeold, time_t *mtimenew)
-{
-	alpm_list_t *files = NULL;
-	int ret;
-
-	/* make a temp one element list */
-	files = alpm_list_add(files, (char*)filename);
-
-	ret = _alpm_downloadfiles_forreal(servers, localpath,
-			files, mtimeold, mtimenew);
-
-	/* free list (data was NOT duplicated) */
-	alpm_list_free(files);
-	return(ret);
 }
 
-int _alpm_download_files(alpm_list_t *files,
+static char *get_destfile(const char *path, const char *filename) {
-		alpm_list_t *servers, const char *localpath)
+	char *destfile;
-{
+	/* len = localpath len + filename len + null */
-	int ret;
+	int len = strlen(path) + strlen(filename) + 1;
+	CALLOC(destfile, len, sizeof(char), RET_ERR(PM_ERR_MEMORY, NULL));
+	snprintf(destfile, len, "%s%s", path, filename);
 
-	ret = _alpm_downloadfiles_forreal(servers, localpath,
+	return(destfile);
-			files, 0, NULL);
-
-	return(ret);
 }
 
+static char *get_tempfile(const char *path, const char *filename) {
+	char *tempfile;
+	/* len = localpath len + filename len + '.part' len + null */
+	int len = strlen(path) + strlen(filename) + 6;
+	CALLOC(tempfile, len, sizeof(char), RET_ERR(PM_ERR_MEMORY, NULL));
+	snprintf(tempfile, len, "%s%s.part", path, filename);
 
-/*
+	return(tempfile);
- * This is the real downloadfiles, used directly by sync_synctree() to check
+}
- * modtimes on remote files.
- *   - if mtime1 is non-NULL, then only download files if they are different
- *     than mtime1.
- *   - if *mtime2 is non-NULL, it will be filled with the mtime of the remote
- *     file.
- *
- * RETURN:  0 for successful download
- *          1 if the mtimes are identical
- *         -1 on error
- */
-int _alpm_downloadfiles_forreal(alpm_list_t *servers, const char *localpath,
-	alpm_list_t *files, time_t mtime1, time_t *mtime2)
-{
-	int dl_thisfile = 0;
-	alpm_list_t *lp;
-	alpm_list_t *complete = NULL;
-	alpm_list_t *i;
-	int ret = -1;
-	char *pkgname = NULL;
 
-	ALPM_LOG_FUNC;
+static int download_internal(const char *url, const char *localpath,
-
+		time_t mtimeold, time_t *mtimenew) {
-	if(files == NULL) {
-		return(0);
-	}
-
-	for(i = servers; i; i = i->next) {
-		const char *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) {
-				goto cleanup;
-			}
-
-			/* pass the raw filename for passing to the callback function */
-			FREE(pkgname);
-			STRDUP(pkgname, fn, (void)0);
-			_alpm_log(PM_LOG_DEBUG, "using '%s' for download progress\n", pkgname);
-
-			snprintf(realfile, PATH_MAX, "%s%s", localpath, fn);
-			snprintf(output, PATH_MAX, "%s%s.part", localpath, fn);
-
-			if(alpm_list_find_str(complete, fn)) {
-				continue;
-			}
-
-			if(!handle->xfercommand
-					|| !strcmp(fileurl->scheme, "file")) {
 	FILE *dlf, *localf = NULL;
 	struct url_stat ust;
 	struct stat st;
 	int chk_resume = 0;
+	int dl_thisfile = 0;
+	char *tempfile, *destfile, *filename;
+	int ret = 0;
+	struct url *fileurl = url_for_string(url);
 
-				if(stat(output, &st) == 0 && st.st_size > 0) {
+	if(!fileurl) {
+		return(-1);
+	}
+
+	filename = get_filename(url);
+	if(!filename) {
+		return(-1);
+	}
+	destfile = get_destfile(localpath, filename);
+	tempfile = get_tempfile(localpath, filename);
+
+	/* pass the raw filename for passing to the callback function */
+	_alpm_log(PM_LOG_DEBUG, "using '%s' for download progress\n", filename);
+
+	if(stat(tempfile, &st) == 0 && st.st_size > 0) {
 		_alpm_log(PM_LOG_DEBUG, "existing file found, using it\n");
 		fileurl->offset = (off_t)st.st_size;
 		dl_thisfile = st.st_size;
-					localf = fopen(output, "a");
+		localf = fopen(tempfile, "ab");
 		chk_resume = 1;
 	} else {
 		fileurl->offset = (off_t)0;
@@ -192,33 +138,21 @@ int _alpm_downloadfiles_forreal(alpm_list_t *servers, const char *localpath,
 			host = fileurl->host;
 		}
 		_alpm_log(PM_LOG_ERROR, _("failed retrieving file '%s' from %s : %s\n"),
-										fn, host, downloadLastErrString);
+				filename, host, downloadLastErrString);
-					if(localf != NULL) {
+		ret = -1;
-						fclose(localf);
+		goto cleanup;
-					}
-					/* try the next server */
-					downloadFreeURL(fileurl);
-					continue;
 	} else {
 		_alpm_log(PM_LOG_DEBUG, "connected to %s successfully\n", fileurl->host);
 	}
 
-				if(ust.mtime && mtime1 && ust.mtime == mtime1) {
+	if(ust.mtime && mtimeold && ust.mtime == mtimeold) {
-					_alpm_log(PM_LOG_DEBUG, "mtimes are identical, skipping %s\n", fn);
+		_alpm_log(PM_LOG_DEBUG, "mtimes are identical, skipping %s\n", filename);
-					complete = alpm_list_add(complete, fn);
-					if(localf != NULL) {
-						fclose(localf);
-					}
-					if(dlf != NULL) {
-						fclose(dlf);
-					}
-					downloadFreeURL(fileurl);
 		ret = 1;
 		goto cleanup;
 	}
 
-				if(ust.mtime && mtime2) {
+	if(ust.mtime && mtimenew) {
-					*mtime2 = ust.mtime;
+		*mtimenew = ust.mtime;
 	}
 
 	if(chk_resume && fileurl->offset == 0) {
@@ -230,23 +164,20 @@ int _alpm_downloadfiles_forreal(alpm_list_t *servers, const char *localpath,
 	}
 
 	if(localf == NULL) {
-					_alpm_rmrf(output);
+		_alpm_rmrf(tempfile);
 		fileurl->offset = (off_t)0;
 		dl_thisfile = 0;
-					localf = fopen(output, "w");
+		localf = fopen(tempfile, "wb");
 		if(localf == NULL) { /* still null? */
-						_alpm_log(PM_LOG_ERROR, _("cannot write to file '%s'\n"), output);
+			_alpm_log(PM_LOG_ERROR, _("cannot write to file '%s'\n"), tempfile);
-						if(dlf != NULL) {
+			ret = -1;
-							fclose(dlf);
-						}
-						downloadFreeURL(fileurl);
 			goto cleanup;
 		}
 	}
 
 	/* Progress 0 - initialize */
 	if(handle->dlcb) {
-					handle->dlcb(pkgname, 0, ust.size);
+		handle->dlcb(filename, 0, ust.size);
 	}
 
 	int nread = 0;
@@ -254,10 +185,8 @@ int _alpm_downloadfiles_forreal(alpm_list_t *servers, const char *localpath,
 	while((nread = fread(buffer, 1, PM_DLBUF_LEN, dlf)) > 0) {
 		if(ferror(dlf)) {
 			_alpm_log(PM_LOG_ERROR, _("error downloading '%s': %s\n"),
-											fn, downloadLastErrString);
+					filename, downloadLastErrString);
-						fclose(localf);
+			ret = -1;
-						fclose(dlf);
-						downloadFreeURL(fileurl);
 			goto cleanup;
 		}
 
@@ -266,43 +195,58 @@ int _alpm_downloadfiles_forreal(alpm_list_t *servers, const char *localpath,
 			nwritten += fwrite(buffer, 1, (nread - nwritten), localf);
 			if(ferror(localf)) {
 				_alpm_log(PM_LOG_ERROR, _("error writing to file '%s': %s\n"),
-												realfile, strerror(errno));
+						destfile, strerror(errno));
-							fclose(localf);
+				ret = -1;
-							fclose(dlf);
-							downloadFreeURL(fileurl);
 				goto cleanup;
 			}
 		}
-
-					if(nwritten != nread) {
-
-					}
 		dl_thisfile += nread;
 
 		if(handle->dlcb) {
-						handle->dlcb(pkgname, dl_thisfile, ust.size);
+			handle->dlcb(filename, dl_thisfile, ust.size);
 		}
 	}
-
+	/* probably safer to close the file descriptors now before renaming the file,
-				downloadFreeURL(fileurl);
+	 * for example to make sure the buffers are flushed.
+	 */
 	fclose(localf);
+	localf = NULL;
 	fclose(dlf);
-				rename(output, realfile);
+	dlf = NULL;
-				complete = alpm_list_add(complete, fn);
+
-			} else {
+	rename(tempfile, destfile);
-				int ret;
+	ret = 0;
+
+cleanup:
+	FREE(tempfile);
+	FREE(destfile);
+	if(localf != NULL) {
+		fclose(localf);
+	}
+	if(dlf != NULL) {
+		fclose(dlf);
+	}
+	downloadFreeURL(fileurl);
+	return(ret);
+}
+
+static int download_external(const char *url, const char *localpath,
+		time_t mtimeold, time_t *mtimenew) {
+	int ret = 0;
+	int retval;
 	int usepart = 0;
 	char *ptr1, *ptr2;
 	char origCmd[PATH_MAX];
 	char parsedCmd[PATH_MAX] = "";
-				char url[PATH_MAX];
 	char cwd[PATH_MAX];
+	char *destfile, *tempfile, *filename;
 
-				/* build the full download url */
+	filename = get_filename(url);
-				snprintf(url, PATH_MAX, "%s://%s%s", fileurl->scheme,
+	if(!filename) {
-						fileurl->host, fileurl->doc);
+		return(-1);
-				/* we don't need this anymore */
+	}
-				downloadFreeURL(fileurl);
+	destfile = get_destfile(localpath, filename);
+	tempfile = get_tempfile(localpath, filename);
 
 	/* replace all occurrences of %o with fn.part */
 	strncpy(origCmd, handle->xfercommand, sizeof(origCmd));
@@ -311,7 +255,7 @@ int _alpm_downloadfiles_forreal(alpm_list_t *servers, const char *localpath,
 		usepart = 1;
 		ptr2[0] = '\0';
 		strcat(parsedCmd, ptr1);
-					strcat(parsedCmd, output);
+		strcat(parsedCmd, tempfile);
 		ptr1 = ptr2 + 2;
 	}
 	strcat(parsedCmd, ptr1);
@@ -331,38 +275,121 @@ int _alpm_downloadfiles_forreal(alpm_list_t *servers, const char *localpath,
 	if(chdir(localpath)) {
 		_alpm_log(PM_LOG_WARNING, _("could not chdir to %s\n"), localpath);
 		pm_errno = PM_ERR_CONNECT_FAILED;
+		ret = -1;
 		goto cleanup;
 	}
 	/* execute the parsed command via /bin/sh -c */
 	_alpm_log(PM_LOG_DEBUG, "running command: %s\n", parsedCmd);
-				ret = system(parsedCmd);
+	retval = system(parsedCmd);
-				if(ret == -1) {
+
+	if(retval == -1) {
 		_alpm_log(PM_LOG_WARNING, _("running XferCommand: fork failed!\n"));
 		pm_errno = PM_ERR_FORK_FAILED;
-					goto cleanup;
+		ret = -1;
-				} else if(ret != 0) {
+	} else if(retval != 0) {
 		/* download failed */
-					_alpm_log(PM_LOG_DEBUG, "XferCommand command returned non-zero status code (%d)\n", ret);
+		_alpm_log(PM_LOG_DEBUG, "XferCommand command returned non-zero status "
+				"code (%d)\n", retval);
+		ret = -1;
 	} else {
 		/* download was successful */
-					complete = alpm_list_add(complete, fn);
 		if(usepart) {
-						rename(output, realfile);
+			rename(tempfile, destfile);
 		}
-				}
-				chdir(cwd);
-			}
-		}
-
-		if(alpm_list_count(complete) == alpm_list_count(files)) {
 		ret = 0;
-			goto cleanup;
-		}
 	}
 
 cleanup:
-	FREE(pkgname);
+	chdir(cwd);
-	alpm_list_free(complete);
+	if(ret == -1) {
+		/* hack to let an user the time to cancel a download */
+		sleep(2);
+	}
+	FREE(destfile);
+	FREE(tempfile);
+
+	return(ret);
+}
+
+static int download(const char *url, const char *localpath,
+		time_t mtimeold, time_t *mtimenew) {
+	int ret;
+	const char *proto = "file://";
+	int len = strlen(proto);
+	if(strncmp(url, proto, len) == 0) {
+		/* we can simply grab an absolute path from the file:// url by starting
+		 * our path at the char following the proto (the root '/')
+		 */
+		const char *sourcefile = url + len;
+		const char *filename = get_filename(url);
+		const char *destfile = get_destfile(localpath, filename);
+
+		if(_alpm_copyfile(sourcefile, destfile) == 0) {
+			return(0);
+		} else {
+			return(-1);
+		}
+	}
+
+	if(handle->xfercommand == NULL) {
+		ret = download_internal(url, localpath, mtimeold, mtimenew);
+	} else {
+		ret = download_external(url, localpath, mtimeold, mtimenew);
+	}
+	return(ret);
+}
+
+/*
+ * Download a single file
+ *   - if mtimeold is non-NULL, then only download the file if it's different
+ *     than mtimeold.
+ *   - if *mtimenew is non-NULL, it will be filled with the mtime of the remote
+ *     file.
+ *
+ * RETURN:  0 for successful download
+ *          1 if the mtimes are identical
+ *         -1 on error
+ */
+int _alpm_download_single_file(const char *filename,
+		alpm_list_t *servers, const char *localpath,
+		time_t mtimeold, time_t *mtimenew)
+{
+	alpm_list_t *i;
+	int ret = -1;
+
+	for(i = servers; i; i = i->next) {
+		const char *server = i->data;
+		char *fileurl = NULL;
+		int len;
+
+		/* print server + filename into a buffer */
+		len = strlen(server) + strlen(filename) + 2;
+		CALLOC(fileurl, len, sizeof(char), RET_ERR(PM_ERR_MEMORY, -1));
+		snprintf(fileurl, len, "%s/%s", server, filename);
+
+		ret = download(fileurl, localpath, mtimeold, mtimenew);
+		FREE(fileurl);
+		if(ret != -1) {
+			break;
+		}
+	}
+
+	return(ret);
+}
+
+int _alpm_download_files(alpm_list_t *files,
+		alpm_list_t *servers, const char *localpath)
+{
+	int ret = 0;
+	alpm_list_t *lp;
+
+	for(lp = files; lp; lp = lp->next) {
+		char *filename = lp->data;
+		if(_alpm_download_single_file(filename, servers,
+					localpath, 0, NULL) == -1) {
+			ret++;
+		}
+	}
 
 	return(ret);
 }
@@ -374,19 +401,20 @@ cleanup:
  */
 char SYMEXPORT *alpm_fetch_pkgurl(const char *url)
 {
-	/* TODO this method will not work at all right now */
 	char *filename, *filepath;
 	const char *cachedir;
+	int ret;
 
 	ALPM_LOG_FUNC;
 
-	filename = NULL;
+	filename = get_filename(url);
 
 	/* find a valid cache dir to download to */
 	cachedir = _alpm_filecache_setup();
 
 	/* download the file */
-	if(_alpm_download_single_file(NULL, NULL, cachedir, 0, NULL)) {
+	ret = download(url, cachedir, 0, NULL);
+	if(ret == -1) {
 		_alpm_log(PM_LOG_WARNING, _("failed to download %s\n"), url);
 		return(NULL);
 	}