In the sync code, we explicitly allocated a string for this field, while in the dload code itself it was filled in with a pointer to another string. This led to a memory leak in the sync download case. Make remote_name non-const and always explicitly allocate it. This patch ensures this as well as uses malloc + snprintf (rather than calloc) in several codepaths, and eliminates the only use of PATH_MAX in the download code. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/dload.c | 26 ++++++++++++++++---------- lib/libalpm/dload.h | 2 +- lib/libalpm/sync.c | 2 +- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index 6f0139d..1e8db1e 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -60,7 +60,7 @@ static char *get_fullpath(const char *path, const char *filename, char *filepath; /* len = localpath len + filename len + suffix len + null */ size_t len = strlen(path) + strlen(filename) + strlen(suffix) + 1; - CALLOC(filepath, len, sizeof(char), return NULL); + MALLOC(filepath, len, return NULL); snprintf(filepath, len, "%s%s%s", path, filename, suffix); return filepath; @@ -279,22 +279,27 @@ static FILE *create_tempfile(struct dload_payload *payload, const char *localpat { int fd; FILE *fp; - char randpath[PATH_MAX]; - alpm_handle_t *handle = payload->handle; + char *randpath; + size_t len; /* create a random filename, which is opened with O_EXCL */ - snprintf(randpath, PATH_MAX, "%salpmtmp.XXXXXX", localpath); + len = strlen(localpath) + 14 + 1; + MALLOC(randpath, len, RET_ERR(payload->handle, ALPM_ERR_MEMORY, NULL)); + snprintf(randpath, len, "%salpmtmp.XXXXXX", localpath); if((fd = mkstemp(randpath)) == -1 || !(fp = fdopen(fd, payload->tempfile_openmode))) { unlink(randpath); close(fd); - _alpm_log(handle, ALPM_LOG_ERROR, + _alpm_log(payload->handle, ALPM_LOG_ERROR, _("failed to create temporary file for download\n")); return NULL; } /* fp now points to our alpmtmp.XXXXXX */ - STRDUP(payload->tempfile_name, randpath, RET_ERR(handle, ALPM_ERR_MEMORY, NULL)); - payload->remote_name = strrchr(randpath, '/') + 1; + free(payload->tempfile_name); + payload->tempfile_name = randpath; + free(payload->remote_name); + STRDUP(payload->remote_name, strrchr(randpath, '/') + 1, + RET_ERR(payload->handle, ALPM_ERR_MEMORY, NULL)); return fp; } @@ -318,7 +323,7 @@ static int curl_download_internal(struct dload_payload *payload, payload->tempfile_openmode = "wb"; if(!payload->remote_name) { - payload->remote_name = get_filename(payload->fileurl); + payload->remote_name = strdup(get_filename(payload->fileurl)); } if(!payload->remote_name || curl_gethost(payload->fileurl, hostname) != 0) { _alpm_log(handle, ALPM_LOG_ERROR, _("url '%s' is invalid\n"), payload->fileurl); @@ -591,10 +596,11 @@ char SYMEXPORT *alpm_fetch_pkgurl(alpm_handle_t *handle, const char *url) void _alpm_dload_payload_free(struct dload_payload *payload) { ASSERT(payload, return); - FREE(payload->fileurl); - FREE(payload->content_disp_name); + FREE(payload->remote_name); FREE(payload->tempfile_name); FREE(payload->destfile_name); + FREE(payload->content_disp_name); + FREE(payload->fileurl); FREE(payload); } diff --git a/lib/libalpm/dload.h b/lib/libalpm/dload.h index bc5eb13..765c5fd 100644 --- a/lib/libalpm/dload.h +++ b/lib/libalpm/dload.h @@ -27,8 +27,8 @@ struct dload_payload { alpm_handle_t *handle; - const char *remote_name; const char *tempfile_openmode; + char *remote_name; char *tempfile_name; char *destfile_name; char *content_disp_name; diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 04ea7d2..8c50ec8 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -874,7 +874,7 @@ static int download_files(alpm_handle_t *handle, alpm_list_t **deltas) /* print server + filename into a buffer */ len = strlen(server_url) + strlen(payload->remote_name) + 2; - 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", server_url, payload->remote_name); payload->handle = handle; payload->allow_resume = 1; -- 1.7.6.4