moparisthebest
/
pacman
mirror of https://github.com/moparisthebest/pacman
1
0
Fork 0
Code Issues Releases Wiki Activity

Refactor download payload reset and free 

This was done to squash a memory leak in the sync database download
code. When we downloaded a database and then reused the payload struct,
we could find ourselves calling get_fullpath() for the signatures and
overwriting non-freed values we had left over from the database
download.

Refactor the payload_free function into a payload_reset function that we
can call that does NOT free the payload itself, so we can reuse payload
structs. This also allows us to move the payload to the stack in some
call paths, relieving us of the need to alloc space.

Signed-off-by: Dan McGee <dan@archlinux.org>
This commit is contained in:
Dan McGee 2011-09-28 12:58:43 -05:00
parent 9a58d5c6c5
commit e0acf2f144
4 changed files with 50 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
lib/libalpm/be_sync.c
View File

@ -201,24 +201,25 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
for(i = db->servers; i; i = i->next) { for(i = db->servers; i; i = i->next) {
const char *server = i->data; const char *server = i->data;
struct dload_payload *payload; struct dload_payload payload;
size_t len; size_t len;
int sig_ret = 0; int sig_ret = 0;
CALLOC(payload, 1, sizeof(*payload), RET_ERR(handle, ALPM_ERR_MEMORY, -1)); memset(&payload, 0, sizeof(struct dload_payload));
/* set hard upper limit of 25MiB */ /* set hard upper limit of 25MiB */
payload->max_size = 25 * 1024 * 1024; payload.max_size = 25 * 1024 * 1024;
/* print server + filename into a buffer (leave space for .sig) */ /* print server + filename into a buffer */
len = strlen(server) + strlen(db->treename) + 9; len = strlen(server) + strlen(db->treename) + 5;
CALLOC(payload->fileurl, len, sizeof(char), RET_ERR(handle, ALPM_ERR_MEMORY, -1)); MALLOC(payload.fileurl, len, RET_ERR(handle, ALPM_ERR_MEMORY, -1));
snprintf(payload->fileurl, len, "%s/%s.db", server, db->treename); snprintf(payload.fileurl, len, "%s/%s.db", server, db->treename);
payload->handle = handle; payload.handle = handle;
payload->force = force; payload.force = force;
payload->unlink_on_fail = 1; payload.unlink_on_fail = 1;
ret = _alpm_download(payload, syncpath, NULL); ret = _alpm_download(&payload, syncpath, NULL);
_alpm_dload_payload_reset(&payload);
if(ret == 0 && (level & ALPM_SIG_DATABASE)) { if(ret == 0 && (level & ALPM_SIG_DATABASE)) {
/* an existing sig file is no good at this point */ /* an existing sig file is no good at this point */
@ -231,20 +232,23 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
free(sigpath); free(sigpath);
/* if we downloaded a DB, we want the .sig from the same server */ /* if we downloaded a DB, we want the .sig from the same server */
snprintf(payload->fileurl, len, "%s/%s.db.sig", server, db->treename); /* print server + filename into a buffer (leave space for .sig) */
payload->handle = handle; len = strlen(server) + strlen(db->treename) + 9;
payload->force = 1; MALLOC(payload.fileurl, len, RET_ERR(handle, ALPM_ERR_MEMORY, -1));
payload->errors_ok = (level & ALPM_SIG_DATABASE_OPTIONAL); snprintf(payload.fileurl, len, "%s/%s.db.sig", server, db->treename);
payload.handle = handle;
payload.force = 1;
payload.errors_ok = (level & ALPM_SIG_DATABASE_OPTIONAL);
/* set hard upper limit of 16KiB */ /* set hard upper limit of 16KiB */
payload->max_size = 16 * 1024; payload.max_size = 16 * 1024;
sig_ret = _alpm_download(payload, syncpath, NULL); sig_ret = _alpm_download(&payload, syncpath, NULL);
/* errors_ok suppresses error messages, but not the return code */ /* errors_ok suppresses error messages, but not the return code */
sig_ret = payload->errors_ok ? 0 : sig_ret; sig_ret = payload.errors_ok ? 0 : sig_ret;
_alpm_dload_payload_reset(&payload);
} }
_alpm_dload_payload_free(payload);
if(ret != -1 && sig_ret != -1) { if(ret != -1 && sig_ret != -1) {
break; break;
} }

48
lib/libalpm/dload.c
View File

@ -390,7 +390,7 @@ static int curl_download_internal(struct dload_payload *payload,
case CURLE_OK: case CURLE_OK:
/* get http/ftp response code */ /* get http/ftp response code */
curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &respcode); curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &respcode);
if(respcode >=400) { if(respcode >= 400) {
payload->unlink_on_fail = 1; payload->unlink_on_fail = 1;
goto cleanup; goto cleanup;
} }
@ -546,7 +546,7 @@ char SYMEXPORT *alpm_fetch_pkgurl(alpm_handle_t *handle, const char *url)
char *filepath; char *filepath;
const char *cachedir; const char *cachedir;
char *final_file = NULL; char *final_file = NULL;
struct dload_payload *payload; struct dload_payload payload;
int ret; int ret;
CHECK_HANDLE(handle, return NULL); CHECK_HANDLE(handle, return NULL);
@ -555,15 +555,17 @@ char SYMEXPORT *alpm_fetch_pkgurl(alpm_handle_t *handle, const char *url)
/* find a valid cache dir to download to */ /* find a valid cache dir to download to */
cachedir = _alpm_filecache_setup(handle); cachedir = _alpm_filecache_setup(handle);
CALLOC(payload, 1, sizeof(*payload), RET_ERR(handle, ALPM_ERR_MEMORY, NULL)); memset(&payload, 0, sizeof(struct dload_payload));
payload->handle = handle; payload.handle = handle;
STRDUP(payload->fileurl, url, RET_ERR(handle, ALPM_ERR_MEMORY, NULL)); STRDUP(payload.fileurl, url, RET_ERR(handle, ALPM_ERR_MEMORY, NULL));
payload->allow_resume = 1; payload.allow_resume = 1;
/* download the file */ /* download the file */
ret = _alpm_download(payload, cachedir, &final_file); ret = _alpm_download(&payload, cachedir, &final_file);
_alpm_dload_payload_reset(&payload);
if(ret == -1) { if(ret == -1) {
_alpm_log(handle, ALPM_LOG_WARNING, _("failed to download %s\n"), url); _alpm_log(handle, ALPM_LOG_WARNING, _("failed to download %s\n"), url);
free(final_file);
return NULL; return NULL;
} }
_alpm_log(handle, ALPM_LOG_DEBUG, "successfully downloaded %s\n", url); _alpm_log(handle, ALPM_LOG_DEBUG, "successfully downloaded %s\n", url);
@ -572,37 +574,37 @@ char SYMEXPORT *alpm_fetch_pkgurl(alpm_handle_t *handle, const char *url)
if(ret == 0 && (handle->siglevel & ALPM_SIG_PACKAGE)) { if(ret == 0 && (handle->siglevel & ALPM_SIG_PACKAGE)) {
char *sig_final_file = NULL; char *sig_final_file = NULL;
size_t len; size_t len;
struct dload_payload *sig_payload;
CALLOC(sig_payload, 1, sizeof(*sig_payload), RET_ERR(handle, ALPM_ERR_MEMORY, NULL));
len = strlen(url) + 5; len = strlen(url) + 5;
CALLOC(sig_payload->fileurl, len, sizeof(char), RET_ERR(handle, ALPM_ERR_MEMORY, NULL)); MALLOC(payload.fileurl, len, RET_ERR(handle, ALPM_ERR_MEMORY, NULL));
snprintf(sig_payload->fileurl, len, "%s.sig", url); snprintf(payload.fileurl, len, "%s.sig", url);
sig_payload->handle = handle; payload.handle = handle;
sig_payload->force = 1; payload.force = 1;
sig_payload->errors_ok = (handle->siglevel & ALPM_SIG_PACKAGE_OPTIONAL); payload.errors_ok = (handle->siglevel & ALPM_SIG_PACKAGE_OPTIONAL);
ret = _alpm_download(sig_payload, cachedir, &sig_final_file); ret = _alpm_download(&payload, cachedir, &sig_final_file);
if(ret == -1 && !sig_payload->errors_ok) { if(ret == -1 && !payload.errors_ok) {
_alpm_log(handle, ALPM_LOG_WARNING, _("failed to download %s\n"), sig_payload->fileurl); _alpm_log(handle, ALPM_LOG_WARNING,
_("failed to download %s\n"), payload.fileurl);
/* Warn now, but don't return NULL. We will fail later during package /* Warn now, but don't return NULL. We will fail later during package
* load time. */ * load time. */
} else if(ret == 0) { } else if(ret == 0) {
_alpm_log(handle, ALPM_LOG_DEBUG, "successfully downloaded %s\n", sig_payload->fileurl); _alpm_log(handle, ALPM_LOG_DEBUG,
"successfully downloaded %s\n", payload.fileurl);
} }
FREE(sig_final_file); FREE(sig_final_file);
_alpm_dload_payload_free(sig_payload); _alpm_dload_payload_reset(&payload);
} }
/* we should be able to find the file the second time around */ /* we should be able to find the file the second time around */
filepath = _alpm_filecache_find(handle, final_file); filepath = _alpm_filecache_find(handle, final_file);
FREE(final_file); free(final_file);
_alpm_dload_payload_free(payload);
return filepath; return filepath;
} }
void _alpm_dload_payload_free(struct dload_payload *payload) { void _alpm_dload_payload_reset(struct dload_payload *payload)
{
ASSERT(payload, return); ASSERT(payload, return);
FREE(payload->remote_name); FREE(payload->remote_name);
@ -610,8 +612,6 @@ void _alpm_dload_payload_free(struct dload_payload *payload) {
FREE(payload->destfile_name); FREE(payload->destfile_name);
FREE(payload->content_disp_name); FREE(payload->content_disp_name);
FREE(payload->fileurl); FREE(payload->fileurl);
FREE(payload);
} }
/* vim: set ts=2 sw=2 noet: */ /* vim: set ts=2 sw=2 noet: */

2
lib/libalpm/dload.h
View File

@ -42,7 +42,7 @@ struct dload_payload {
CURLcode curlerr; /* last error produced by curl */ CURLcode curlerr; /* last error produced by curl */
}; };
void _alpm_dload_payload_free(struct dload_payload *payload); void _alpm_dload_payload_reset(struct dload_payload *payload);
int _alpm_download(struct dload_payload *payload, const char *localpath, int _alpm_download(struct dload_payload *payload, const char *localpath,
char **final_file); char **final_file);

5
lib/libalpm/sync.c
View File

@ -891,9 +891,8 @@ static int download_files(alpm_handle_t *handle, alpm_list_t **deltas)
} }
} }
alpm_list_free_inner(files, (alpm_list_fn_free)_alpm_dload_payload_free); alpm_list_free_inner(files, (alpm_list_fn_free)_alpm_dload_payload_reset);
alpm_list_free(files); FREELIST(files);
files = NULL;
} }
} }