Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/diskspace.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/libalpm/diskspace.c b/lib/libalpm/diskspace.c index dcab3b0..cfd0a7a 100644 --- a/lib/libalpm/diskspace.c +++ b/lib/libalpm/diskspace.c @@ -84,6 +84,9 @@ static int mount_point_load_fsinfo(alpm_handle_t *handle, alpm_mountpoint_t *mou _alpm_log(handle, ALPM_LOG_DEBUG, "loading fsinfo for %s\n", mountpoint->mount_dir); mountpoint->read_only = mountpoint->fsp.f_flag & ST_RDONLY; mountpoint->fsinfo_loaded = MOUNT_FSINFO_LOADED; +#else + (void)handle; + (void)mountpoint; #endif return 0; -- 1.8.5.2

Noticed using clang and `-Wpadded`. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/dload.h | 4 ++-- lib/libalpm/graph.h | 4 ++-- lib/libalpm/handle.h | 4 +++- src/pacman/conf.c | 2 +- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/libalpm/dload.h b/lib/libalpm/dload.h index 6c9f7a7..964c338 100644 --- a/lib/libalpm/dload.h +++ b/lib/libalpm/dload.h @@ -31,6 +31,8 @@ struct dload_payload { char *destfile_name; char *content_disp_name; char *fileurl; + alpm_list_t *servers; + long respcode; off_t initial_size; off_t max_size; off_t prevprogress; @@ -39,11 +41,9 @@ struct dload_payload { int errors_ok; int unlink_on_fail; int trust_remote_name; - alpm_list_t *servers; #ifdef HAVE_LIBCURL CURLcode curlerr; /* last error produced by curl */ #endif - long respcode; }; void _alpm_dload_payload_reset(struct dload_payload *payload); diff --git a/lib/libalpm/graph.h b/lib/libalpm/graph.h index bf06047..8db35bf 100644 --- a/lib/libalpm/graph.h +++ b/lib/libalpm/graph.h @@ -24,12 +24,12 @@ #include "alpm_list.h" typedef struct __alpm_graph_t { - char state; /* 0: untouched, -1: entered, other: leaving time */ - off_t weight; /* weight of the node */ void *data; struct __alpm_graph_t *parent; /* where did we come from? */ alpm_list_t *children; alpm_list_t *childptr; /* points to a child in children list */ + off_t weight; /* weight of the node */ + char state; /* 0: untouched, -1: entered, other: leaving time */ } alpm_graph_t; alpm_graph_t *_alpm_graph_new(void); diff --git a/lib/libalpm/handle.h b/lib/libalpm/handle.h index 4126e1a..b6b8e2b 100644 --- a/lib/libalpm/handle.h +++ b/lib/libalpm/handle.h @@ -55,7 +55,6 @@ struct __alpm_handle_t { alpm_db_t *db_local; /* local db pointer */ alpm_list_t *dbs_sync; /* List of (alpm_db_t *) */ FILE *logstream; /* log file stream pointer */ - int lockfd; /* lock file descriptor */ alpm_trans_t *trans; #ifdef HAVE_LIBCURL @@ -100,6 +99,9 @@ struct __alpm_handle_t { /* error code */ alpm_errno_t pm_errno; + /* lock file descriptor */ + int lockfd; + /* for delta parsing efficiency */ int delta_regex_compiled; regex_t delta_regex; diff --git a/src/pacman/conf.c b/src/pacman/conf.c index cf8a417..258d538 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -746,8 +746,8 @@ struct section_t { int is_options; int parse_options; /* db section option gathering */ - alpm_siglevel_t siglevel; alpm_list_t *servers; + alpm_siglevel_t siglevel; alpm_db_usage_t usage; }; -- 1.8.5.2

This shrinks down the total size of the package struct by a handful of bytes, saving us some memory and cache pressure when we are loading up the entirety of the sync and local databases. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/package.h | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/lib/libalpm/package.h b/lib/libalpm/package.h index 07b043c..9fb83de 100644 --- a/lib/libalpm/package.h +++ b/lib/libalpm/package.h @@ -102,18 +102,6 @@ struct __alpm_pkg_t { off_t isize; off_t download_size; - int scriptlet; - - alpm_pkgreason_t reason; - alpm_pkgvalidation_t validation; - alpm_dbinfrq_t infolevel; - alpm_pkgfrom_t origin; - /* origin == PKG_FROM_FILE, use pkg->origin_data.file - * origin == PKG_FROM_*DB, use pkg->origin_data.db */ - union { - alpm_db_t *db; - char *file; - } origin_data; alpm_handle_t *handle; alpm_list_t *licenses; @@ -131,6 +119,19 @@ struct __alpm_pkg_t { struct pkg_operations *ops; alpm_filelist_t files; + + /* origin == PKG_FROM_FILE, use pkg->origin_data.file + * origin == PKG_FROM_*DB, use pkg->origin_data.db */ + union { + alpm_db_t *db; + char *file; + } origin_data; + + alpm_dbinfrq_t infolevel; + alpm_pkgvalidation_t validation; + alpm_pkgfrom_t origin; + alpm_pkgreason_t reason; + unsigned int scriptlet; }; alpm_file_t *_alpm_file_copy(alpm_file_t *dest, const alpm_file_t *src); -- 1.8.5.2

When calling open(), use O_CLOEXEC as much as possible to ensure the file descriptor is closed when and if a process using libalpm forks. For most of these cases, and especially in utility functions, the file descriptor is opened and closed in the same function, so we don't have too much to worry about. However, for things like the log file and database lock file, we should ensure descriptors aren't left hanging around for children to touch. This patch is inspired by the problem in FS#36161, where an open file descriptor to the current working directory prevents chroot() from working on FreeBSD. We don't need this file descriptor in the child process, so open it (and now several others) with O_CLOEXEC. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/add.c | 2 +- lib/libalpm/handle.c | 2 +- lib/libalpm/log.c | 11 +++++++++-- lib/libalpm/util.c | 14 +++++++------- 4 files changed, 18 insertions(+), 11 deletions(-) diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c index 214e78e..fa8da4d 100644 --- a/lib/libalpm/add.c +++ b/lib/libalpm/add.c @@ -556,7 +556,7 @@ static int commit_single_pkg(alpm_handle_t *handle, alpm_pkg_t *newpkg, } /* save the cwd so we can restore it later */ - OPEN(cwdfd, ".", O_RDONLY); + OPEN(cwdfd, ".", O_RDONLY | O_CLOEXEC); if(cwdfd < 0) { _alpm_log(handle, ALPM_LOG_ERROR, _("could not get current working directory\n")); } diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c index 1d661a2..f30597d 100644 --- a/lib/libalpm/handle.c +++ b/lib/libalpm/handle.c @@ -110,7 +110,7 @@ int _alpm_handle_lock(alpm_handle_t *handle) FREE(dir); do { - handle->lockfd = open(handle->lockfile, O_WRONLY | O_CREAT | O_EXCL, 0000); + handle->lockfd = open(handle->lockfile, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, 0000); } while(handle->lockfd == -1 && errno == EINTR); return (handle->lockfd >= 0 ? 0 : -1); diff --git a/lib/libalpm/log.c b/lib/libalpm/log.c index 271bd00..183ff22 100644 --- a/lib/libalpm/log.c +++ b/lib/libalpm/log.c @@ -49,9 +49,16 @@ int SYMEXPORT alpm_logaction(alpm_handle_t *handle, const char *prefix, /* check if the logstream is open already, opening it if needed */ if(handle->logstream == NULL) { - handle->logstream = fopen(handle->logfile, "a"); + int fd; + do { + fd = open(handle->logfile, O_WRONLY | O_APPEND | O_CREAT | O_CLOEXEC, + 0000); + } while(fd == -1 && errno == EINTR); + if(fd >= 0) { + handle->logstream = fdopen(fd, "a"); + } /* if we couldn't open it, we have an issue */ - if(handle->logstream == NULL) { + if(fd < 0 || handle->logstream == NULL) { if(errno == EACCES) { handle->pm_errno = ALPM_ERR_BADPERMS; } else if(errno == ENOENT) { diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index 40a5ebd..7ceabf2 100644 --- a/lib/libalpm/util.c +++ b/lib/libalpm/util.c @@ -153,9 +153,9 @@ int _alpm_copyfile(const char *src, const char *dest) MALLOC(buf, (size_t)ALPM_BUFFER_SIZE, return 1); - OPEN(in, src, O_RDONLY); + OPEN(in, src, O_RDONLY | O_CLOEXEC); do { - out = open(dest, O_WRONLY | O_CREAT, 0000); + out = open(dest, O_WRONLY | O_CREAT | O_BINARY | O_CLOEXEC, 0000); } while(out == -1 && errno == EINTR); if(in < 0 || out < 0) { goto cleanup; @@ -244,7 +244,7 @@ int _alpm_open_archive(alpm_handle_t *handle, const char *path, archive_read_support_format_all(*archive); _alpm_log(handle, ALPM_LOG_DEBUG, "opening archive %s\n", path); - OPEN(fd, path, O_RDONLY); + OPEN(fd, path, O_RDONLY | O_CLOEXEC); if(fd < 0) { _alpm_log(handle, ALPM_LOG_ERROR, _("could not open file %s: %s\n"), path, strerror(errno)); @@ -326,7 +326,7 @@ int _alpm_unpack(alpm_handle_t *handle, const char *path, const char *prefix, oldmask = umask(0022); /* save the cwd so we can restore it later */ - OPEN(cwdfd, ".", O_RDONLY); + OPEN(cwdfd, ".", O_RDONLY | O_CLOEXEC); if(cwdfd < 0) { _alpm_log(handle, ALPM_LOG_ERROR, _("could not get current working directory\n")); } @@ -502,7 +502,7 @@ int _alpm_run_chroot(alpm_handle_t *handle, const char *cmd, char *const argv[]) int retval = 0; /* save the cwd so we can restore it later */ - OPEN(cwdfd, ".", O_RDONLY); + OPEN(cwdfd, ".", O_RDONLY | O_CLOEXEC); if(cwdfd < 0) { _alpm_log(handle, ALPM_LOG_ERROR, _("could not get current working directory\n")); } @@ -778,7 +778,7 @@ static int md5_file(const char *path, unsigned char output[16]) MALLOC(buf, (size_t)ALPM_BUFFER_SIZE, return 1); - OPEN(fd, path, O_RDONLY); + OPEN(fd, path, O_RDONLY | O_CLOEXEC); if(fd < 0) { free(buf); return 1; @@ -820,7 +820,7 @@ static int sha2_file(const char *path, unsigned char output[32], int is224) MALLOC(buf, (size_t)ALPM_BUFFER_SIZE, return 1); - OPEN(fd, path, O_RDONLY); + OPEN(fd, path, O_RDONLY | O_CLOEXEC); if(fd < 0) { free(buf); return 1; -- 1.8.5.2

On 03/01/14 04:37, Dan McGee wrote:

Two issues have snuck in that prevent the compile from working.

Signed-off-by: Dan McGee <dan@archlinux.org> ---

This and the rest of the patches available on my 'random-fixes' branch.

lib/libalpm/sync.c | 2 +- src/pacman/package.c | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index e358585..9531fa2 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -1225,7 +1225,7 @@ int _alpm_sync_commit(alpm_handle_t *handle, alpm_list_t **data) return -1; }

-#if HAVE_LIBGPGME +#ifdef HAVE_LIBGPGME

I not that #if and #ifdef are both used in many places. $ git grep "#if HAVE_" lib/libalpm/signing.c:#if HAVE_LIBGPGME lib/libalpm/signing.c:#if HAVE_LIBGPGME lib/libalpm/signing.c:#if HAVE_LIBGPGME lib/libalpm/sync.c:#if HAVE_LIBGPGME m4/acinclude.m4:#if HAVE_SYS_UCRED_H Should all these change?

/* make sure all required signatures are in keyring */ if(check_keyring(handle)) { return -1; diff --git a/src/pacman/package.c b/src/pacman/package.c index 52219ff..c44696b 100644 --- a/src/pacman/package.c +++ b/src/pacman/package.c @@ -193,6 +193,10 @@ void dump_pkg_full(alpm_pkg_t *pkg, int extra) }

if(from == ALPM_PKG_FROM_SYNCDB && extra) { + string_display(_("MD5 Sum :"), alpm_pkg_get_md5sum(pkg), cols); + string_display(_("SHA-256 Sum :"), alpm_pkg_get_sha256sum(pkg), cols); + +#ifdef HAVE_LIBGPGME

I went for the approach of exposing alpm_decode_signature even if GPGME is unavailable, because it does not require any GPGME functions and it could still be useful for a frontend: https://patchwork.archlinux.org/patch/1790/

const char *base64_sig = alpm_pkg_get_base64_sig(pkg); alpm_list_t *keys = NULL; if(base64_sig) { @@ -204,10 +208,8 @@ void dump_pkg_full(alpm_pkg_t *pkg, int extra) } else { keys = alpm_list_add(keys, _("None")); } - - string_display(_("MD5 Sum :"), alpm_pkg_get_md5sum(pkg), cols); - string_display(_("SHA-256 Sum :"), alpm_pkg_get_sha256sum(pkg), cols); list_display(_("Signatures :"), keys, cols); +#endif } else { list_display(_("Validated By :"), validation, cols); }