On Sat, Apr 4, 2009 at 3:30 AM, Sebastian Nowicki<sebnow@gmail.com> wrote:
I fixed the warnings and errors and tested it a bit more. Everything seems to be working fine. Using dynamic allocation for the xfercommand string removed the problem with PATH_MAX. The previously inlined functions are now separate functions as before, with minor changes due to the lack of macros from libalpm (see diff below).
So I finally got around to this. I made a few small changes, detailed in the diff below, that I included in the final patch. diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 31e1f45..ce8c691 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -151,9 +151,6 @@ void alpm_option_add_ignoregrp(const char *grp); void alpm_option_set_ignoregrps(alpm_list_t *ignoregrps); int alpm_option_remove_ignoregrp(const char *grp); -const char *alpm_option_get_xfercommand(); -void alpm_option_set_xfercommand(const char *cmd); -
You simply forgot to kill these here.
unsigned short alpm_option_get_nopassiveftp(); void alpm_option_set_nopassiveftp(unsigned short nopasv); void alpm_option_set_usedelta(unsigned short usedelta); diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index 9aa6141..6b163ce 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -267,12 +267,17 @@ static int download(const char *url, const char *localpath, time_t mtimeold, time_t *mtimenew) { if(handle->fetchcb == NULL) { #if defined(INTERNAL_DOWNLOAD) - handle->fetchcb = download_internal; + return(download_internal(url, localpath, mtimeold, mtimenew)); #else RET_ERR(PM_ERR_EXTERNAL_DOWNLOAD, -1); #endif + } else { + int ret = handle->fetchcb(url, localpath, mtimeold, mtimenew); + if(ret == -1) { + RET_ERR(PM_ERR_EXTERNAL_DOWNLOAD, -1); + } + return(ret); } - return handle->fetchcb(url, localpath, mtimeold, mtimenew); }
Structuring the code this way ensures the correct pm_errno value will propagate up the call stack.
/* diff --git a/src/pacman/conf.h b/src/pacman/conf.h index 2bf952a..2d3de98 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -73,7 +73,7 @@ typedef struct __config_t { unsigned short cleanmethod; /* select -Sc behavior */ alpm_list_t *holdpkg; alpm_list_t *syncfirst; - char *xfercommand; /* external download command */ + char *xfercommand; } config_t; /* Operations */ diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 7cf1396..7cecd90 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -657,7 +657,7 @@ int download_with_xfercommand(const char *url, const char *localpath, /* cwd to the download directory */ getcwd(cwd, PATH_MAX); if(chdir(localpath)) { - pm_printf(PM_LOG_DEBUG, "could not chdir to %s\n", localpath); + pm_printf(PM_LOG_WARNING, "could not chdir to %s\n", localpath); ret = -1; goto cleanup; } @@ -666,7 +666,7 @@ int download_with_xfercommand(const char *url, const char *localpath, retval = system(parsedCmd); if(retval == -1) { - pm_printf(PM_LOG_DEBUG, "running XferCommand: fork failed!\n"); + pm_printf(PM_LOG_WARNING, "running XferCommand: fork failed!\n");
These two were warnings before; they are kind of a big deal.
ret = -1; } else if(retval != 0) { /* download failed */ -Dan