On 06/05/16 at 07:51pm, Tobias Stoeckmann wrote:
> Some resources (memory or file descriptors) are not released on all
> error paths.
>
> Signed-off-by: Tobias Stoeckmann <[email protected]>
> ---
> Yes it's rather ironic to send this patch after forgetting one on
> my own just now. ;)
> ---
> lib/libalpm/add.c | 5 ++++-
> lib/libalpm/backup.c | 5 +++--
> lib/libalpm/be_local.c | 18 +++++++++++++++---
> lib/libalpm/be_package.c | 1 +
> 4 files changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
> index f5c9a95..d132e52 100644
> --- a/lib/libalpm/add.c
> +++ b/lib/libalpm/add.c
> @@ -466,7 +466,7 @@ static int commit_single_pkg(alpm_handle_t *handle,
> alpm_pkg_t *newpkg,
> }
> }
>
> - /* prepare directory for database entries so permission are correct
> after
> + /* prepare directory for database entries so permissions are correct
> after
> changelog/install script installation */
> if(_alpm_local_db_prepare(db, newpkg)) {
> alpm_logaction(handle, ALPM_CALLER_PREFIX,
> @@ -503,6 +503,9 @@ static int commit_single_pkg(alpm_handle_t *handle,
> alpm_pkg_t *newpkg,
> _alpm_log(handle, ALPM_LOG_ERROR, _("could not change
> directory to %s (%s)\n"),
> handle->root, strerror(errno));
> _alpm_archive_read_free(archive);
> + if(cwdfd >= 0) {
> + close(cwdfd);
> + }
> close(fd);
> ret = -1;
> goto cleanup;
> diff --git a/lib/libalpm/backup.c b/lib/libalpm/backup.c
> index f622589..50bad5e 100644
> --- a/lib/libalpm/backup.c
> +++ b/lib/libalpm/backup.c
> @@ -48,9 +48,10 @@ int _alpm_split_backup(const char *string, alpm_backup_t
> **backup)
> ptr++;
> /* now str points to the filename and ptr points to the hash */
> STRDUP((*backup)->name, str, FREE(str); return -1);
> - STRDUP((*backup)->hash, ptr, FREE(str); return -1);
> + STRDUP((*backup)->hash, ptr, FREE((*backup)->name); FREE(str); return
> -1);
> FREE(str);
> - return 0;}
> + return 0;
> +}
>
> /* Look for a filename in a alpm_pkg_t.backup list. If we find it,
> * then we return the full backup entry.
> diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c
> index f817822..0b351f9 100644
> --- a/lib/libalpm/be_local.c
> +++ b/lib/libalpm/be_local.c
> @@ -794,7 +794,7 @@ static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t
> inforeq)
> _alpm_strip_newline(line, 0);
> if(strcmp(line, "%FILES%") == 0) {
> size_t files_count = 0, files_size = 0, len;
> - alpm_file_t *files = NULL;
> + alpm_file_t *files = NULL, *newfiles;
>
> while(safe_fgets(line, sizeof(line), fp) &&
> (len =
> _alpm_strip_newline(line, 0))) {
> @@ -805,13 +805,18 @@ static int local_db_read(alpm_pkg_t *info,
> alpm_dbinfrq_t inforeq)
Immediately above this is a call to _alpm_greedy_grow which needs
memory released on failure as well.
> /* since we know the length of the file
> string already,
> * we can do malloc + memcpy rather
> than strdup */
> len += 1;
> - MALLOC(files[files_count].name, len,
> goto error);
> + MALLOC(files[files_count].name, len,
> goto nomem);
> memcpy(files[files_count].name, line,
> len);
> files_count++;
> }
> /* attempt to hand back any memory we don't
> need */
> if(files_count > 0) {
> - files = realloc(files,
> sizeof(alpm_file_t) * files_count);
> + newfiles = realloc(files,
> sizeof(alpm_file_t) * files_count);
newfiles should be declared here; we limit variables to the smallest
scope possible.
> + if(newfiles == NULL) {
> + goto nomem;
I don't think we need to treat this as an error. The realloc is just
releasing memory; failure should not be significant.
> + }
> + files = newfiles;
> +
> /* make sure the list is sorted */
> qsort(files, files_count,
> sizeof(alpm_file_t), _alpm_files_cmp);
> } else {
> @@ -819,6 +824,13 @@ static int local_db_read(alpm_pkg_t *info,
> alpm_dbinfrq_t inforeq)
> }
> info->files.count = files_count;
> info->files.files = files;
> + continue;
> +nomem:
> + while(files_count > 0) {
> + FREE(files[--files_count].name);
> + }
> + FREE(files);
> + goto error;
> } else if(strcmp(line, "%BACKUP%") == 0) {
> while(safe_fgets(line, sizeof(line), fp) &&
> _alpm_strip_newline(line, 0)) {
> alpm_backup_t *backup;
> diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c
> index c9ed770..a79c0c5 100644
> --- a/lib/libalpm/be_package.c
> +++ b/lib/libalpm/be_package.c
> @@ -762,6 +762,7 @@ int SYMEXPORT alpm_pkg_load(alpm_handle_t *handle, const
> char *filename, int ful
>
> if(fail) {
> _alpm_log(handle, ALPM_LOG_ERROR, _("required
> key missing from keyring\n"));
> + free(sigpath);
> return -1;
> }
> }
> --
> 2.8.3
