[pacman-dev] [PATCH 1/2] Use STRDUP for error checking in more places
Use STRDUP() over strdup() to catch memory allocation errors. There are still some instances of strdup left, but these are in functions that currently have no error path and would require a larger rework. Signed-off-by: Allan McRae <allan@archlinux.org> --- lib/libalpm/be_package.c | 10 +++++++--- lib/libalpm/handle.c | 2 +- lib/libalpm/util.c | 3 ++- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index 4832966b..38ba365d 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -201,11 +201,15 @@ static int parse_descfile(alpm_handle_t *handle, struct archive *a, alpm_pkg_t * } else if(strcmp(key, "pkgdesc") == 0) { STRDUP(newpkg->desc, ptr, return -1); } else if(strcmp(key, "group") == 0) { - newpkg->groups = alpm_list_add(newpkg->groups, strdup(ptr)); + char *tmp = NULL; + STRDUP(tmp, ptr, return -1); + newpkg->groups = alpm_list_add(newpkg->groups, tmp); } else if(strcmp(key, "url") == 0) { STRDUP(newpkg->url, ptr, return -1); } else if(strcmp(key, "license") == 0) { - newpkg->licenses = alpm_list_add(newpkg->licenses, strdup(ptr)); + char *tmp = NULL; + STRDUP(tmp, ptr, return -1); + newpkg->licenses = alpm_list_add(newpkg->licenses, tmp); } else if(strcmp(key, "builddate") == 0) { newpkg->builddate = _alpm_parsedate(ptr); } else if(strcmp(key, "packager") == 0) { @@ -660,7 +664,7 @@ alpm_pkg_t *_alpm_pkg_load_internal(alpm_handle_t *handle, /* internal fields for package struct */ newpkg->origin = ALPM_PKG_FROM_FILE; - newpkg->origin_data.file = strdup(pkgfile); + STRDUP(newpkg->origin_data.file, pkgfile, goto error); newpkg->ops = get_file_pkg_ops(); newpkg->handle = handle; newpkg->infolevel = INFRQ_BASE | INFRQ_DESC | INFRQ_SCRIPTLET; diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c index 8ae08d11..e3b2c911 100644 --- a/lib/libalpm/handle.c +++ b/lib/libalpm/handle.c @@ -105,7 +105,7 @@ int _alpm_handle_lock(alpm_handle_t *handle) ASSERT(handle->lockfd < 0, return 0); /* create the dir of the lockfile first */ - dir = strdup(handle->lockfile); + STRDUP(dir, handle->lockfile, return -1); ptr = strrchr(dir, '/'); if(ptr) { *ptr = '\0'; diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index cebc87ec..cb838e43 100644 --- a/lib/libalpm/util.c +++ b/lib/libalpm/util.c @@ -350,7 +350,8 @@ int _alpm_unpack(alpm_handle_t *handle, const char *path, const char *prefix, /* If specific files were requested, skip entries that don't match. */ if(list) { - char *entry_prefix = strdup(entryname); + char *entry_prefix = NULL; + STRDUP(entry_prefix, entryname, ret = 1; goto cleanup); char *p = strstr(entry_prefix,"/"); if(p) { *(p + 1) = '\0'; -- 2.25.1
The GOTO_ERR define was added in commit 80ae8014 for use in future commits. There are plenty of places in the code base it can be used, so convert them. Signed-off-by: Allan McRae <allan@archlinux.org> --- lib/libalpm/be_local.c | 6 ++---- lib/libalpm/be_package.c | 24 ++++++++---------------- lib/libalpm/be_sync.c | 6 ++---- lib/libalpm/dload.c | 6 ++---- lib/libalpm/signing.c | 16 ++++++---------- lib/libalpm/sync.c | 5 ++--- 6 files changed, 22 insertions(+), 41 deletions(-) diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c index 689f0102..e73a97bb 100644 --- a/lib/libalpm/be_local.c +++ b/lib/libalpm/be_local.c @@ -256,8 +256,7 @@ static struct archive *_cache_mtree_open(alpm_pkg_t *pkg) } if((mtree = archive_read_new()) == NULL) { - pkg->handle->pm_errno = ALPM_ERR_LIBARCHIVE; - goto error; + GOTO_ERR(pkg->handle, ALPM_ERR_LIBARCHIVE, error); } _alpm_archive_read_support_filter_all(mtree); @@ -266,9 +265,8 @@ static struct archive *_cache_mtree_open(alpm_pkg_t *pkg) if((r = _alpm_archive_read_open_file(mtree, mtfile, ALPM_BUFFER_SIZE))) { _alpm_log(pkg->handle, ALPM_LOG_ERROR, _("error while reading file %s: %s\n"), mtfile, archive_error_string(mtree)); - pkg->handle->pm_errno = ALPM_ERR_LIBARCHIVE; _alpm_archive_read_free(mtree); - goto error; + GOTO_ERR(pkg->handle, ALPM_ERR_LIBARCHIVE, error); } free(mtfile); diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index 38ba365d..f98832f4 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -459,8 +459,7 @@ static int build_filelist_from_mtree(alpm_handle_t *handle, alpm_pkg_t *pkg, str /* create a new archive to parse the mtree and load it from archive into memory */ /* TODO: split this into a function */ if((mtree = archive_read_new()) == NULL) { - handle->pm_errno = ALPM_ERR_LIBARCHIVE; - goto error; + GOTO_ERR(handle, ALPM_ERR_LIBARCHIVE, error); } _alpm_archive_read_support_filter_all(mtree); @@ -479,8 +478,7 @@ static int build_filelist_from_mtree(alpm_handle_t *handle, alpm_pkg_t *pkg, str if(size < 0) { _alpm_log(handle, ALPM_LOG_DEBUG, _("error while reading package %s: %s\n"), pkg->filename, archive_error_string(archive)); - handle->pm_errno = ALPM_ERR_LIBARCHIVE; - goto error; + GOTO_ERR(handle, ALPM_ERR_LIBARCHIVE, error); } if(size == 0) { break; @@ -493,8 +491,7 @@ static int build_filelist_from_mtree(alpm_handle_t *handle, alpm_pkg_t *pkg, str _alpm_log(handle, ALPM_LOG_DEBUG, _("error while reading mtree of package %s: %s\n"), pkg->filename, archive_error_string(mtree)); - handle->pm_errno = ALPM_ERR_LIBARCHIVE; - goto error; + GOTO_ERR(handle, ALPM_ERR_LIBARCHIVE, error); } while((ret = archive_read_next_header(mtree, &mtree_entry)) == ARCHIVE_OK) { @@ -517,8 +514,7 @@ static int build_filelist_from_mtree(alpm_handle_t *handle, alpm_pkg_t *pkg, str if(ret != ARCHIVE_EOF && ret != ARCHIVE_OK) { /* An error occurred */ _alpm_log(handle, ALPM_LOG_DEBUG, _("error while reading mtree of package %s: %s\n"), pkg->filename, archive_error_string(mtree)); - handle->pm_errno = ALPM_ERR_LIBARCHIVE; - goto error; + GOTO_ERR(handle, ALPM_ERR_LIBARCHIVE, error); } /* throw away any files we loaded directly from the archive */ @@ -583,11 +579,9 @@ alpm_pkg_t *_alpm_pkg_load_internal(alpm_handle_t *handle, newpkg = _alpm_pkg_new(); if(newpkg == NULL) { - handle->pm_errno = ALPM_ERR_MEMORY; - goto error; + GOTO_ERR(handle, ALPM_ERR_MEMORY, error); } - STRDUP(newpkg->filename, pkgfile, - handle->pm_errno = ALPM_ERR_MEMORY; goto error); + STRDUP(newpkg->filename, pkgfile, GOTO_ERR(handle, ALPM_ERR_MEMORY, error)); newpkg->size = st.st_size; _alpm_log(handle, ALPM_LOG_DEBUG, "starting package load for %s\n", pkgfile); @@ -637,8 +631,7 @@ alpm_pkg_t *_alpm_pkg_load_internal(alpm_handle_t *handle, if(archive_read_data_skip(archive)) { _alpm_log(handle, ALPM_LOG_ERROR, _("error while reading package %s: %s\n"), pkgfile, archive_error_string(archive)); - handle->pm_errno = ALPM_ERR_LIBARCHIVE; - goto error; + GOTO_ERR(handle, ALPM_ERR_LIBARCHIVE, error); } /* if we are not doing a full read, see if we have all we need */ @@ -650,8 +643,7 @@ alpm_pkg_t *_alpm_pkg_load_internal(alpm_handle_t *handle, if(ret != ARCHIVE_EOF && ret != ARCHIVE_OK) { /* An error occurred */ _alpm_log(handle, ALPM_LOG_ERROR, _("error while reading package %s: %s\n"), pkgfile, archive_error_string(archive)); - handle->pm_errno = ALPM_ERR_LIBARCHIVE; - goto error; + GOTO_ERR(handle, ALPM_ERR_LIBARCHIVE, error); } if(!config) { diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c index 5f457122..2cee97e0 100644 --- a/lib/libalpm/be_sync.c +++ b/lib/libalpm/be_sync.c @@ -464,9 +464,8 @@ static int sync_db_populate(alpm_db_t *db) db->pkgcache = _alpm_pkghash_create(est_count); if(db->pkgcache == NULL) { - db->handle->pm_errno = ALPM_ERR_MEMORY; ret = -1; - goto cleanup; + GOTO_ERR(db->handle, ALPM_ERR_MEMORY, cleanup); } while((archive_ret = archive_read_next_header(archive, &entry)) == ARCHIVE_OK) { @@ -485,9 +484,8 @@ static int sync_db_populate(alpm_db_t *db) _alpm_log(db->handle, ALPM_LOG_ERROR, _("could not read db '%s' (%s)\n"), db->treename, archive_error_string(archive)); _alpm_db_free_pkgcache(db); - db->handle->pm_errno = ALPM_ERR_LIBARCHIVE; ret = -1; - goto cleanup; + GOTO_ERR(db->handle, ALPM_ERR_LIBARCHIVE, cleanup); } count = alpm_list_count(db->pkgcache->list); diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index 670da03d..57232a53 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -424,11 +424,10 @@ static int curl_download_internal(struct dload_payload *payload, if(localf == NULL) { localf = fopen(payload->tempfile_name, payload->tempfile_openmode); if(localf == NULL) { - handle->pm_errno = ALPM_ERR_RETRIEVE; _alpm_log(handle, ALPM_LOG_ERROR, _("could not open file %s: %s\n"), payload->tempfile_name, strerror(errno)); - goto cleanup; + GOTO_ERR(handle, ALPM_ERR_RETRIEVE, cleanup); } } @@ -535,10 +534,9 @@ static int curl_download_internal(struct dload_payload *payload, * as actually being transferred during curl_easy_perform() */ if(!DOUBLE_EQ(remote_size, -1) && !DOUBLE_EQ(bytes_dl, -1) && !DOUBLE_EQ(bytes_dl, remote_size)) { - handle->pm_errno = ALPM_ERR_RETRIEVE; _alpm_log(handle, ALPM_LOG_ERROR, _("%s appears to be truncated: %jd/%jd bytes\n"), payload->remote_name, (intmax_t)bytes_dl, (intmax_t)remote_size); - goto cleanup; + GOTO_ERR(handle, ALPM_ERR_RETRIEVE, cleanup); } if(payload->trust_remote_name) { diff --git a/lib/libalpm/signing.c b/lib/libalpm/signing.c index 52c27ccb..c8eaaca2 100644 --- a/lib/libalpm/signing.c +++ b/lib/libalpm/signing.c @@ -586,16 +586,14 @@ int _alpm_gpgme_checksig(alpm_handle_t *handle, const char *path, || (sigfile = fopen(sigpath, "rb")) == NULL) { _alpm_log(handle, ALPM_LOG_DEBUG, "sig path %s could not be opened\n", sigpath); - handle->pm_errno = ALPM_ERR_SIG_MISSING; - goto error; + GOTO_ERR(handle, ALPM_ERR_SIG_MISSING, error); } } /* does the file we are verifying exist? */ file = fopen(path, "rb"); if(file == NULL) { - handle->pm_errno = ALPM_ERR_NOT_A_FILE; - goto error; + GOTO_ERR(handle, ALPM_ERR_NOT_A_FILE, error); } if(init_gpgme(handle)) { @@ -619,8 +617,7 @@ int _alpm_gpgme_checksig(alpm_handle_t *handle, const char *path, int decode_ret = alpm_decode_signature(base64_sig, &decoded_sigdata, &data_len); if(decode_ret) { - handle->pm_errno = ALPM_ERR_SIG_INVALID; - goto gpg_error; + GOTO_ERR(handle, ALPM_ERR_SIG_INVALID, error); } gpg_err = gpgme_data_new_from_mem(&sigdata, (char *)decoded_sigdata, data_len, 0); @@ -637,15 +634,14 @@ int _alpm_gpgme_checksig(alpm_handle_t *handle, const char *path, CHECK_ERR(); if(!verify_result || !verify_result->signatures) { _alpm_log(handle, ALPM_LOG_DEBUG, "no signatures returned\n"); - handle->pm_errno = ALPM_ERR_SIG_MISSING; - goto gpg_error; + GOTO_ERR(handle, ALPM_ERR_SIG_MISSING, gpg_error); } for(gpgsig = verify_result->signatures, sigcount = 0; gpgsig; gpgsig = gpgsig->next, sigcount++); _alpm_log(handle, ALPM_LOG_DEBUG, "%d signatures returned\n", sigcount); CALLOC(siglist->results, sigcount, sizeof(alpm_sigresult_t), - handle->pm_errno = ALPM_ERR_MEMORY; goto gpg_error); + GOTO_ERR(handle, ALPM_ERR_MEMORY, gpg_error)); siglist->count = sigcount; for(gpgsig = verify_result->signatures, sigcount = 0; gpgsig; @@ -682,7 +678,7 @@ int _alpm_gpgme_checksig(alpm_handle_t *handle, const char *path, gpg_err = GPG_ERR_NO_ERROR; /* we dupe the fpr in this case since we have no key to point at */ STRDUP(result->key.fingerprint, gpgsig->fpr, - handle->pm_errno = ALPM_ERR_MEMORY; goto gpg_error); + GOTO_ERR(handle, ALPM_ERR_MEMORY, gpg_error)); } else { CHECK_ERR(); if(key->uids) { diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 50b21b54..8a9dcae8 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -456,11 +456,10 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t **data) } } else { /* pm_errno was set by resolvedeps, callback may have overwrote it */ - handle->pm_errno = ALPM_ERR_UNSATISFIED_DEPS; alpm_list_free(resolved); alpm_list_free(unresolvable); ret = -1; - goto cleanup; + GOTO_ERR(handle, ALPM_ERR_UNSATISFIED_DEPS, cleanup); } } @@ -820,7 +819,7 @@ static int download_files(alpm_handle_t *handle) const alpm_pkg_t *pkg = i->data; struct dload_payload payload = {0}; - STRDUP(payload.remote_name, pkg->filename, handle->pm_errno = ALPM_ERR_MEMORY; goto finish); + STRDUP(payload.remote_name, pkg->filename, GOTO_ERR(handle, ALPM_ERR_MEMORY, finish)); payload.servers = pkg->origin_data.db->servers; payload.max_size = pkg->size; -- 2.25.1
participants (1)
-
Allan McRae