July 2013 - Pacman-dev - lists.archlinux.org

[pacman-dev] [PATCH] Group optional dependencies by installation status
by Drew DeVault 12 Jul '13

12 Jul '13

Not sure how expensive alpm_db_get_pkg is, so it might be worthwhile to use something other than make_optstring to accomplish this if we want to avoid calling it four times per package. Signed-off-by: Drew DeVault <sir(a)cmpwn.com> --- src/pacman/util.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/pacman/util.c b/src/pacman/util.c index 23c4009..c1d3dab 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1243,10 +1243,19 @@ void display_optdepends(alpm_pkg_t *pkg) optdeps = alpm_pkg_get_optdepends(pkg); - /* turn optdepends list into a text list */ + /* turn optdepends list into a text list, with uninstalled dependencies last */ for(i = optdeps; i; i = alpm_list_next(i)) { alpm_depend_t *optdep = i->data; - optstrings = alpm_list_add(optstrings, make_optstring(optdep)); + if(alpm_db_get_pkg(alpm_get_localdb(config->handle), optdep->name) != 0) { + optstrings = alpm_list_add(optstrings, make_optstring(optdep)); + } + } + + for(i = optdeps; i; i = alpm_list_next(i)) { + alpm_depend_t *optdep = i->data; + if(alpm_db_get_pkg(alpm_get_localdb(config->handle), optdep->name) == 0) { + optstrings = alpm_list_add(optstrings, make_optstring(optdep)); + } } if(optstrings) { -- 1.8.3.2

3 9

[pacman-dev] Patch for FS#31557
by Ashley Whetter 11 Jul '13

11 Jul '13

Hey everyone, I've made a fix for issue FS#31557 (Merge installing makedepends and checkdepends). I've attached the patch and pasted it below: >From 447ba301bd9d24d3b60dd02d6382f64965ad7878 Mon Sep 17 00:00:00 2001 From: Ashley Whetter <awhetter.2011(a)my.bristol.ac.uk> Date: Wed, 10 Jul 2013 22:44:55 +0100 Subject: [PATCH] makedeps and checkdeps are now installed together Fixes FS#31557 Signed-off-by: Ashley Whetter <awhetter.2011(a)my.bristol.ac.uk> --- scripts/makepkg.sh.in | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 2a39157..4de0d34 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -2952,11 +2952,12 @@ else fi msg "$(gettext "Checking buildtime dependencies...")" - resolve_deps ${makedepends[@]} || deperr=1 + build_depends=("${makedepends[@]}") if (( CHECKFUNC )); then - resolve_deps ${checkdepends[@]} || deperr=1 + build_depends+=("${checkdepends[@]}") fi + resolve_deps ${build_depends[@]} || deperr=1 if (( RMDEPS )); then current_pkglist=($(run_pacman -Qq)) # required by remove_deps -- 1.8.3.2 Regards, Ashley

5 6

[pacman-dev] [PATCH 0/5] Use more auto in autotools
by Allan McRae 10 Jul '13

10 Jul '13

The use of autotools in pacman is interesting... We commit most autotools generated files, but not configure or Makefile. One reason for this is that we needed to patch ltmain.sh to properly handle -Wl,-as-needed, because the Arch package maintainer refused to do so unless upstream accepts! Libtool upstream have known about the bug since at least 2006, so that does not look hopeful. The new approach is to have autogen.sh call autoreconf and then have it patch the ltmain.sh file. This removes a lot of crap that should be autogenerated from our git repo: 38 files changed, 83 insertions(+), 28256 deletions(-) Allan McRae (5): Remove autotools files Update gitignore files for use with autoreconf Use autoreconf in autogen.sh Patch ltmain on autoreconf Remove autotools files with autoclean.sh .gitignore | 1 + autoclean.sh | 18 +- autogen.sh | 6 +- build-aux/.gitignore | 2 + build-aux/compile | 347 -- build-aux/config.guess | 1558 ------- build-aux/config.rpath | 614 --- build-aux/config.sub | 1788 -------- build-aux/depcomp | 791 ---- build-aux/install-sh | 527 --- build-aux/ltmain-asneeded.patch | 33 + build-aux/ltmain.sh | 9672 --------------------------------------- build-aux/missing | 215 - build-aux/mkinstalldirs | 162 - configure.ac | 2 +- lib/libalpm/po/.gitignore | 10 +- lib/libalpm/po/Makefile.in.in | 444 -- m4/.gitignore | 2 + m4/gettext.m4 | 383 -- m4/gpgme.m4 | 307 -- m4/iconv.m4 | 214 - m4/intlmacosx.m4 | 51 - m4/lib-ld.m4 | 110 - m4/lib-link.m4 | 774 ---- m4/lib-prefix.m4 | 224 - m4/libtool.m4 | 7831 ------------------------------- m4/ltoptions.m4 | 369 -- m4/ltsugar.m4 | 123 - m4/ltversion.m4 | 23 - m4/lt~obsolete.m4 | 98 - m4/nls.m4 | 32 - m4/pkg.m4 | 159 - m4/po.m4 | 449 -- m4/progtest.m4 | 92 - scripts/po/.gitignore | 10 +- scripts/po/Makefile.in.in | 444 -- src/pacman/po/.gitignore | 10 +- src/pacman/po/Makefile.in.in | 444 -- 38 files changed, 83 insertions(+), 28256 deletions(-) create mode 100644 build-aux/.gitignore delete mode 100755 build-aux/compile delete mode 100755 build-aux/config.guess delete mode 100755 build-aux/config.rpath delete mode 100755 build-aux/config.sub delete mode 100755 build-aux/depcomp delete mode 100755 build-aux/install-sh create mode 100644 build-aux/ltmain-asneeded.patch delete mode 100644 build-aux/ltmain.sh delete mode 100755 build-aux/missing delete mode 100755 build-aux/mkinstalldirs delete mode 100644 lib/libalpm/po/Makefile.in.in create mode 100644 m4/.gitignore delete mode 100644 m4/gettext.m4 delete mode 100644 m4/gpgme.m4 delete mode 100644 m4/iconv.m4 delete mode 100644 m4/intlmacosx.m4 delete mode 100644 m4/lib-ld.m4 delete mode 100644 m4/lib-link.m4 delete mode 100644 m4/lib-prefix.m4 delete mode 100644 m4/libtool.m4 delete mode 100644 m4/ltoptions.m4 delete mode 100644 m4/ltsugar.m4 delete mode 100644 m4/ltversion.m4 delete mode 100644 m4/lt~obsolete.m4 delete mode 100644 m4/nls.m4 delete mode 100644 m4/pkg.m4 delete mode 100644 m4/po.m4 delete mode 100644 m4/progtest.m4 delete mode 100644 scripts/po/Makefile.in.in delete mode 100644 src/pacman/po/Makefile.in.in -- 1.8.3.1

4 19

[pacman-dev] [PATCH 1/2] Add missing header
by Allan McRae 10 Jul '13

10 Jul '13

Exposed when building with --without-libcurl Signed-off-by: Allan McRae <allan(a)archlinux.org> --- lib/libalpm/be_sync.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c index feda6f5..bd68739 100644 --- a/lib/libalpm/be_sync.c +++ b/lib/libalpm/be_sync.c @@ -22,6 +22,7 @@ #include <sys/types.h> #include <sys/stat.h> #include <fcntl.h> +#include <limits.h> #include <unistd.h> /* libarchive */ -- 1.8.3.2

1 1

[pacman-dev] [PATCH] be_sync: avoid crashing on files in the root of a DB
by Dave Reisner 09 Jul '13

09 Jul '13

As seen: https://bbs.archlinux.org/viewtopic.php?pid=1297766 Signed-off-by: Dave Reisner <dreisner(a)archlinux.org> --- lib/libalpm/be_sync.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c index feda6f5..f3e0a33 100644 --- a/lib/libalpm/be_sync.c +++ b/lib/libalpm/be_sync.c @@ -561,6 +561,13 @@ static int sync_db_read(alpm_db_t *db, struct archive *archive, return -1; } + if(filename == NULL) { + /* A file exists outside of a subdirectory. This isn't a read error, so return + * success and try to continue on. */ + _alpm_log(db->handle, ALPM_LOG_DEBUG, "unknown database file: %s\n", filename); + return 0; + } + if(strcmp(filename, "desc") == 0 || strcmp(filename, "depends") == 0 || (strcmp(filename, "deltas") == 0 && db->handle->deltaratio > 0.0) ) { int ret; -- 1.8.3.2

4 6

[pacman-dev] [PATCH 0/2] separate ini parsing from pacman/conf.c
by Sascha Kruse 06 Jul '13

06 Jul '13

This set of patches separate the ini parsing from pacman/conf.c into its own file to make the code reusable for future additions, like parsing config files for hooks. The config file gets parsed into a list of ini_section_t which contains the name of the section and a list of directives which in turn contain key, value, file and linenum. File and linenum are included to keep the ability to mention the location of errors in pacmans error messages. I realize that a new release is planed for the near future and this patch is not intended to be included in that release, since it adds new strings to be translated and it probably needs more than one review cycle. Best regards, Sascha Kruse

4 7

[pacman-dev] (no subject)
by Jonathan Frazier 05 Jul '13

05 Jul '13

Subject: re: [pacman-dev] [PATCH] pacdiff using pacman db v2 Reply-To: In-Reply-To: 51D1468A.3030209(a)xinu.at On 07/01/13 at 11:06am, Florian Pritz wrote: > Subject: [pacman-dev] [PATCH] pacdiff using pacman db v2 > > Please move that v2 into the PATCH tag like so: [PATCH v2]. Otherwise > it sounds like you use "version 2 of the pacman db" and it might end > up in the final commit. Ok will do. > > I'd also suggest to change the subject to something like "pacdiff: use > pacman db for lookups to increase performance" so it's more obvious > what this patch does and why. Still missing the "why is using that > faster/better than locate", but that can be in the commit message. > > On 01.07.2013 02:00, Jonathan Frazier wrote: > > Implement the default search using the local pacman db. > > I'm not sure the default should switch to an inferior method (inferior > because it doesn't find everything). updatedb might be too slow on > certain hardware or whatever, but I think that's a minority so we > should stick with locate. Also that way long time users won't have > left over .pacsaves. > First let me say that I am not stuck on this being the default option, but it is the one I will use. I could say locate is inferior because it finds pacsaves not in use. giving more noise and less signal. My rational is more that it finds the active configs more throughly. Speed is not the only factor. I will try and add more descriptive commit msgs. > > Use @sysconfdir@/pacman.conf to find the local db Search for > > pacsave.0 files. > > You also search for .pacsave, .pacsave.1, ..., .pacnew, .pacorig. No > need to mention this explicitly here. Also the comment about > pacman.conf doesn't belong here unless you explain why you do it this > way and you have a good reason not to do anything else. > > Actually none of the 3 lines explain why any of this is done. > > I know these are the changes you made compared to the previous patch, > but such changes belong to the comment section below (between the > "---" marker and the list of changed files) not into the commit > message. > Adding support for these extra pacsave files is a new feature and worth noting in the changelog. this feature was requested in another thread. I can make it a separate patch if the rest of this is going to take longer. > All I remember (now) is that you do this because updatedb is slow on > some low end hardware, but that should really be mentioned here > because I might forget (or am I already wrong? I honestly don't know) > and others won't even have known in the first place and someone might > care when they look through the commit log in a few years to find why > a certain feature appeared (been there, done that). > I will try to be more descriptive in my changlog. > > > > Signed-off-by: Jonathan Frazier <eyeswide(a)gmail.com> --- > > contrib/pacdiff.sh.in | 86 > > ++++++++++++++++++++++++++++++++++++++------------- 1 file changed, > > 64 insertions(+), 22 deletions(-) > > > > diff --git a/contrib/pacdiff.sh.in b/contrib/pacdiff.sh.in index > > 47779d6..ee80c9c 100644 --- a/contrib/pacdiff.sh.in +++ > > b/contrib/pacdiff.sh.in @@ -21,9 +21,8 @@ declare -r > > myname='pacdiff' declare -r myver='@PACKAGE_VERSION@' > > > > -diffprog=${DIFFPROG:-vimdiff} > > -diffsearchpath=${DIFFSEARCHPATH:-/etc} -locate=0 > > +diffprog=${DIFF_PROG:-vimdiff} > > +difffindpath="${DIFF_FIND_PATH:-/etc}" > > Out of scope of this patch. > DIFFSEARCHPATH is misleading and I find it difficult to read. the old name implies it works for all search types, when it only applied to find. Since nobody is defending find as the default, it should be changed along with the default. Adding underscores was just cause I am fixing it. Is DIFFFINDPATH more acceptable? > > USE_COLOR='y' > > > > m4_include(../scripts/library/output_format.sh) @@ -32,11 +31,19 @@ > > m4_include(../scripts/library/term_colors.sh) > > > > usage() { echo "$myname : a simple pacnew/pacorig/pacsave updater" > > - echo "Usage : $myname [-l]" - echo " -l/--locate makes $myname > > use locate rather than find" - echo " DIFFPROG variable allows to > > override the default vimdiff" - echo " DIFFSEARCHPATH allows to > > override the default /etc path" - echo "Example : DIFFPROG=meld > > DIFFSEARCHPATH=\"/boot /etc /usr\" $myname" + echo "" + echo > > "Usage : $myname [-lf]" + echo " -l/--locate makes $myname search > > using locate" + echo " -f/--find makes $myname search using > > find" > > Add an option for your pacsave method. Maybe -p. > Does it make sense to have an option for the default? I don't think so. Whatever the default ends up being it doesn't need a option. > > + echo "" + echo "Note: the default search looks for backup files in > > the local pacman db" + echo " this generally will not find > > *.pacsave files" + echo "" + echo " DIFF_PROG variable will > > override the default editor: vimdiff" + echo " DIFF_FIND_PATH will > > override the default path when using --find" + echo "" + echo > > "Example : DIFF_PROG=meld $myname" + echo "Example : > > DIFF_FIND_PATH=\"/boot /etc /usr\" $myname -f" > > Out of scope. > > > } > > > > version() { @@ -45,27 +52,61 @@ version() { echo 'Copyright (C) > > 2013 Pacman Development Team <pacman-dev(a)archlinux.org>' } > > > > -cmd() { - if [ $locate -eq 1 ]; then - locate -0 -e -b > > \*.pacnew \*.pacorig \*.pacsave - else - find $diffsearchpath $ > > -name \*.pacnew -o -name \*.pacorig -o -name \*.pacsave $ -print0 - > > fi -} - > > Any reason you moved cmd()? Makes reviewing the changes harder. > I was concerned about the scope of the variables it contains. as none are not defined at that time. I don't remember if it was necessary or just my correcting "bad" style from writing in other languages. > > if [ $# -gt 0 ]; then case $1 in -l|--locate) - locate=1;; + > > locate=$(type -P locate);; + -f|--find) + find=$(type -P > > find);; > > Why did you change that to do a path lookup and not exit if you can't > find it? > I didn't think to test if find or locate could not be found. I can add that. The idea was to only define a variable for the active search type and use that to define the mode later. using the path is convenient. > > -V|--version) > > - version; exit 0;; > > + version; exit 0;; > > -h|--help) > > - usage; exit 0;; > > + usage; exit 0;; > > *) > > - usage; exit 1;; > > + usage; exit 1;; > > esac > > fi > > > > +if [[ ! -n $locate && ! -n $find ]]; then > > Don't do that. Someone might add a 4th option in the future and then > they have to adjust everything again. > > Just use a $mode variable and set it to a default or to the last > specified option. I'd also use strings here for easier understanding > when you use them in conditions ("find", "locate", "pacsave"). > I will add a var for just the pacman db search. which will replace this. > > + if [[ ! -r @sysconfdir@/pacman.conf ]]; then > > + error "unable to read @sysconfdir@/pacman.conf" > > + usage; exit 1 > > + fi > > + > > + eval $(awk '/DBPath/ {print $1$2$3}' @sysconfdir@/pacman.conf) > > Make sysconfdir a variable like myver, don't hardcode it directly. (same > for localstatedir below) > This code is what is used in bacman and pacscripts as others requested I use as an example. What is the reason to make it a variable? The only reason I could come up with for making it variable was to support running in a alternate root. Adding such support seemed overly complex given the current state of pacdiff. I was thinking of adding this feature after converting the option handling to allow options with associated strings. I don't see it as a reason to hold up the rest of this code. > > + pac_db="${DBPath:-@localstatedir@/lib/pacman/}local" > > + if [[ ! -d "${pac_db}" ]]; then > > + error "unable to read pacman db %s". "${pac_db}" > > + usage; exit 1 > > + fi > > +fi > > + > > +check_backup() { > > + [[ -f "$1" ]] && printf "$1"'\0' > > +} > > + > > +cmd() { > > + if [[ -n $locate ]]; then > > + locate -0 -e -b \*.pacnew \*.pacorig \*.pacsave '*.pacsave.[0-9]' > > + elif [[ -n $find ]]; then > > + find "${difffindpath}" $ -name \*.pacnew -o -name \*.pacorig -o -name \*.pacsave -o -name '*.pacsave.[0-9]' $ -print0 2> /dev/null > > .pacsave pattern change is out of scope. This isn't a logical change > given the patch subject. Better to split that into it's own patch > (before this patch so it can be merge independently). > again this was asked for in my previous submission. I will try and document it better. the original subject contained all three notes, so it was excessively long. > > + else > > + # parse local pacman db for backup files and look for pac* based on them > > + awk '/^%BACKUP%$/ { > > + while (getline) { > > + if (/^$/) { nextfile } > > + print $1 > > + } > > + }' "${pac_db}"/*/files | while read -r bkup; do > > + check_backup "/$bkup.pacnew" > > + check_backup "/$bkup.pacorig" > > + for f in "/$bkup.pacsave"{,.{0..9}}; do > > + check_backup "$f" > > + done > > + done > > + fi > > +} > > + > > # see http://mywiki.wooledge.org/BashFAQ/020 > > while IFS= read -u 3 -r -d '' pacfile; do > > file="${pacfile%.pac*}" > > @@ -82,16 +123,17 @@ while IFS= read -u 3 -r -d '' pacfile; do > > msg2 "Files are identical, removing..." > > rm -v "$pacfile" > > else > > - ask "(V)iew, (S)kip, (R)emove %s, (O)verwrite with %s: [v/s/r/o] " "$file_type" "$file_type" > > + ask "(V)iew, (S)kip, (R)emove %s, (O)verwrite with %s, (Q)uit: [v/s/r/o/q] " "$file_type" "$file_type" > > Out of the scope of this patch and IMHO unnecessary (^C is enough). > I agree that q is not necessary, but I find it annoying that it is not there. I have been thanked for adding it when sharing a previous version of this code. At least it shows that that is a reasonable time to quit. /shrug > > while read c; do > > case $c in > > + q|Q) exit 1;; > > The user asked us to quit, not an error. > will fix, thanks > > r|R) rm -v "$pacfile"; break ;; > > o|O) mv -v "$pacfile" "$file"; break ;; > > v|V) > > $diffprog "$pacfile" "$file" > > rm -iv "$pacfile"; break ;; > > s|S) break ;; > > - *) ask "Invalid answer. Try again: [v/s/r/o] "; continue ;; > > + *) ask "Invalid answer. Try again: [v/s/r/o/q] "; continue ;; > > esac > > done > > fi > > > > thanks for the comments. Do you think adding a prompt asking if pacdiff should run updatedb when called with -l is a good idea? Given the confusion brought by my misuse of git format-patch and combining commits into one. If it is preferred I can go back to pushing code to github and ask for further reviews when ready. Jonathan

3 2

[pacman-dev] [GIT] The official pacman repository branch, master, updated. v4.1.2-56-g897e2b1
by allan＠archlinux.org 05 Jul '13

05 Jul '13

This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing the project "The official pacman repository". The branch, master has been updated via 897e2b121a107f6d250d38ff4d1d0a4960885c94 (commit) via eb19d41d5f85f169cee7570f783821cb705ad37a (commit) via 7b8f8753b15037bf4a88126ffde8195416432c72 (commit) from dcb3b9f85ff8761fa1fe7ef01df2d6b011b9cb73 (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email; so we list those revisions in full, below. - Log ----------------------------------------------------------------- commit 897e2b121a107f6d250d38ff4d1d0a4960885c94 Author: Andrew Gregory <andrew.gregory.8(a)gmail.com> Date: Thu Jul 4 06:36:20 2013 -0400 use full path when checking for mountpoints Signed-off-by: Andrew Gregory <andrew.gregory.8(a)gmail.com> Signed-off-by: Allan McRae <allan(a)archlinux.org> commit eb19d41d5f85f169cee7570f783821cb705ad37a Author: Dave Reisner <dreisner(a)archlinux.org> Date: Wed Jul 3 20:33:19 2013 -0400 do not check error from close(2) On operating systems we support, the behavior is always such that the kernel will do the right thing as far as invalidating the file descriptor, regardless of the eventual return value. Therefore, potentially looping and calling close multiple times is wrong. At best, we call close again on an invalid FD and throw a spurious EBADF error. At worst, we might close an FD which doesn't belong to us when a multi-threaded application opens its own file descriptor between iterations of the loop. Signed-off-by: Dave Reisner <dreisner(a)archlinux.org> Signed-off-by: Allan McRae <allan(a)archlinux.org> commit 7b8f8753b15037bf4a88126ffde8195416432c72 Author: Simon Gomizelj <simongmzlj(a)gmail.com> Date: Wed Jul 3 22:34:14 2013 -0400 Consolidate and improve table implementations Implement both the VerbosePkgList and the summary message with the same table. Improve VerbosePkgList by caching attributes and cell's lengths instead of recaculating them. Right align every cell that containing a file size in both the VerbosePkgList and the summary. Simplify the printf statements and the alignment application. Signed-off-by: Simon Gomizelj <simongmzlj(a)gmail.com> Signed-off-by: Allan McRae <allan(a)archlinux.org> ----------------------------------------------------------------------- Summary of changes: lib/libalpm/add.c | 6 +- lib/libalpm/be_package.c | 10 +- lib/libalpm/be_sync.c | 2 +- lib/libalpm/dload.c | 2 +- lib/libalpm/remove.c | 15 ++- lib/libalpm/util.c | 28 +++--- lib/libalpm/util.h | 1 - src/pacman/conf.c | 5 +- src/pacman/util.c | 237 ++++++++++++++++++++++++----------------------- 9 files changed, 160 insertions(+), 146 deletions(-) hooks/post-receive -- The official pacman repository

1 0

[pacman-dev] [PATCH] do not check error from close(2)
by Dave Reisner 04 Jul '13

04 Jul '13

On operating systems we support, the behavior is always such that the kernel will do the right thing as far as invalidating the file descriptor, regardless of the eventual return value. Therefore, potentially looping and calling close multiple times is wrong. At best, we call close again on an invalid FD and throw a spurious EBADF error. At worst, we might close an FD which doesn't belong to us when a multi-threaded application opens its own file descriptor between iterations of the loop. Signed-off-by: Dave Reisner <dreisner(a)archlinux.org> --- This goes further than the current patch on the ML and removes the CLOSE macro defined in alpm as well. lib/libalpm/add.c | 6 +++--- lib/libalpm/be_package.c | 10 +++++----- lib/libalpm/be_sync.c | 2 +- lib/libalpm/dload.c | 2 +- lib/libalpm/util.c | 28 ++++++++++++++-------------- lib/libalpm/util.h | 1 - src/pacman/conf.c | 5 +---- 7 files changed, 25 insertions(+), 29 deletions(-) diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c index c20e7c6..d6dc748 100644 --- a/lib/libalpm/add.c +++ b/lib/libalpm/add.c @@ -550,7 +550,7 @@ 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); - CLOSE(fd); + close(fd); ret = -1; goto cleanup; } @@ -580,7 +580,7 @@ static int commit_single_pkg(alpm_handle_t *handle, alpm_pkg_t *newpkg, errors += extract_single_file(handle, archive, entry, newpkg, oldpkg); } _alpm_archive_read_free(archive); - CLOSE(fd); + close(fd); /* restore the old cwd if we have it */ if(cwdfd >= 0) { @@ -588,7 +588,7 @@ static int commit_single_pkg(alpm_handle_t *handle, alpm_pkg_t *newpkg, _alpm_log(handle, ALPM_LOG_ERROR, _("could not restore working directory (%s)\n"), strerror(errno)); } - CLOSE(cwdfd); + close(cwdfd); } if(errors) { diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index 5a70968..6262621 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -76,7 +76,7 @@ static void *_package_changelog_open(alpm_pkg_t *pkg) if(!changelog) { pkg->handle->pm_errno = ALPM_ERR_MEMORY; _alpm_archive_read_free(archive); - CLOSE(fd); + close(fd); return NULL; } changelog->archive = archive; @@ -86,7 +86,7 @@ static void *_package_changelog_open(alpm_pkg_t *pkg) } /* we didn't find a changelog */ _alpm_archive_read_free(archive); - CLOSE(fd); + close(fd); errno = ENOENT; return NULL; @@ -126,7 +126,7 @@ static int _package_changelog_close(const alpm_pkg_t UNUSED *pkg, void *fp) int ret; struct package_changelog *changelog = fp; ret = _alpm_archive_read_free(changelog->archive); - CLOSE(changelog->fd); + close(changelog->fd); free(changelog); return ret; } @@ -477,7 +477,7 @@ alpm_pkg_t *_alpm_pkg_load_internal(alpm_handle_t *handle, } _alpm_archive_read_free(archive); - CLOSE(fd); + close(fd); /* internal fields for package struct */ newpkg->origin = ALPM_PKG_FROM_FILE; @@ -510,7 +510,7 @@ error: _alpm_pkg_free(newpkg); _alpm_archive_read_free(archive); if(fd >= 0) { - CLOSE(fd); + close(fd); } return NULL; diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c index f9fd5d1..feda6f5 100644 --- a/lib/libalpm/be_sync.c +++ b/lib/libalpm/be_sync.c @@ -474,7 +474,7 @@ static int sync_db_populate(alpm_db_t *db) cleanup: _alpm_archive_read_free(archive); if(fd >= 0) { - CLOSE(fd); + close(fd); } return count; } diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index 7513822..b759c78 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -366,7 +366,7 @@ static FILE *create_tempfile(struct dload_payload *payload, const char *localpat fchmod(fd, ~(_getumask()) & 0666) || !(fp = fdopen(fd, payload->tempfile_openmode))) { unlink(randpath); - CLOSE(fd); + close(fd); _alpm_log(payload->handle, ALPM_LOG_ERROR, _("failed to create temporary file for download\n")); free(randpath); diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index f84fb66..78cfc49 100644 --- a/lib/libalpm/util.c +++ b/lib/libalpm/util.c @@ -186,10 +186,10 @@ int _alpm_copyfile(const char *src, const char *dest) cleanup: free(buf); if(in >= 0) { - CLOSE(in); + close(in); } if(out >= 0) { - CLOSE(out); + close(out); } return ret; } @@ -275,7 +275,7 @@ error: _alpm_archive_read_free(*archive); *archive = NULL; if(fd >= 0) { - CLOSE(fd); + close(fd); } RET_ERR(handle, error, -1); } @@ -394,13 +394,13 @@ int _alpm_unpack(alpm_handle_t *handle, const char *path, const char *prefix, cleanup: umask(oldmask); _alpm_archive_read_free(archive); - CLOSE(fd); + close(fd); if(cwdfd >= 0) { if(fchdir(cwdfd) != 0) { _alpm_log(handle, ALPM_LOG_ERROR, _("could not restore working directory (%s)\n"), strerror(errno)); } - CLOSE(cwdfd); + close(cwdfd); } return ret; @@ -537,12 +537,12 @@ int _alpm_run_chroot(alpm_handle_t *handle, const char *cmd, char *const argv[]) if(pid == 0) { /* this code runs for the child only (the actual chroot/exec) */ - CLOSE(1); - CLOSE(2); + close(1); + close(2); while(dup2(pipefd[1], 1) == -1 && errno == EINTR); while(dup2(pipefd[1], 2) == -1 && errno == EINTR); - CLOSE(pipefd[0]); - CLOSE(pipefd[1]); + close(pipefd[0]); + close(pipefd[1]); /* use fprintf instead of _alpm_log to send output through the parent */ if(chroot(handle->root) != 0) { @@ -564,10 +564,10 @@ int _alpm_run_chroot(alpm_handle_t *handle, const char *cmd, char *const argv[]) int status; FILE *pipe_file; - CLOSE(pipefd[1]); + close(pipefd[1]); pipe_file = fdopen(pipefd[0], "r"); if(pipe_file == NULL) { - CLOSE(pipefd[0]); + close(pipefd[0]); retval = 1; } else { while(!feof(pipe_file)) { @@ -609,7 +609,7 @@ cleanup: _alpm_log(handle, ALPM_LOG_ERROR, _("could not restore working directory (%s)\n"), strerror(errno)); } - CLOSE(cwdfd); + close(cwdfd); } return retval; @@ -784,7 +784,7 @@ static int md5_file(const char *path, unsigned char output[16]) MD5_Update(&ctx, buf, n); } - CLOSE(fd); + close(fd); free(buf); if(n < 0) { @@ -834,7 +834,7 @@ static int sha2_file(const char *path, unsigned char output[32], int is224) } } - CLOSE(fd); + close(fd); free(buf); if(n < 0) { diff --git a/lib/libalpm/util.h b/lib/libalpm/util.h index 56031f3..8e577de 100644 --- a/lib/libalpm/util.h +++ b/lib/libalpm/util.h @@ -88,7 +88,6 @@ void _alpm_alloc_fail(size_t size); #endif #define OPEN(fd, path, flags) do { fd = open(path, flags | O_BINARY); } while(fd == -1 && errno == EINTR) -#define CLOSE(fd) do { int _ret; do { _ret = close(fd); } while(_ret == -1 && errno == EINTR); } while(0) /** * Used as a buffer/state holder for _alpm_archive_fgets(). diff --git a/src/pacman/conf.c b/src/pacman/conf.c index 5adc96c..2985aba 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -257,14 +257,11 @@ static int download_with_xfercommand(const char *url, const char *localpath, cleanup: /* restore the old cwd if we have it */ if(cwdfd >= 0) { - int close_ret; if(fchdir(cwdfd) != 0) { pm_printf(ALPM_LOG_ERROR, _("could not restore working directory (%s)\n"), strerror(errno)); } - do { - close_ret = close(cwdfd); - } while(close_ret == -1 && errno == EINTR); + close(cwdfd); } if(ret == -1) { -- 1.8.3.2

2 1

[pacman-dev] [PATCH] use full path when checking for mountpoints
by Andrew Gregory 04 Jul '13

04 Jul '13

Signed-off-by: Andrew Gregory <andrew.gregory.8(a)gmail.com> --- lib/libalpm/remove.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index 0b4f80c..7cb86ff 100644 --- a/lib/libalpm/remove.c +++ b/lib/libalpm/remove.c @@ -267,6 +267,16 @@ int _alpm_remove_prepare(alpm_handle_t *handle, alpm_list_t **data) return 0; } +/** + * @brief Test if a directory is being used as a mountpoint. + * + * @param handle context handle + * @param directory path to test, must be absolute and include trailing '/' + * @param stbuf stat result for @a directory, may be NULL + * + * @return 0 if @a directory is not a mountpoint or on error, 1 if @a directory + * is a mountpoint + */ static int dir_is_mountpoint(alpm_handle_t *handle, const char *directory, const struct stat *stbuf) { @@ -317,13 +327,14 @@ static int can_remove_file(alpm_handle_t *handle, const alpm_file_t *file, return 1; } + snprintf(filepath, PATH_MAX, "%s%s", handle->root, file->name); + if(file->name[strlen(file->name) - 1] == '/' && - dir_is_mountpoint(handle, file->name, NULL)) { + dir_is_mountpoint(handle, filepath, NULL)) { /* we do not remove mountpoints */ return 1; } - snprintf(filepath, PATH_MAX, "%s%s", handle->root, file->name); /* If we fail write permissions due to a read-only filesystem, abort. * Assume all other possible failures are covered somewhere else */ if(_alpm_access(handle, NULL, filepath, W_OK) == -1) { -- 1.8.3.2

1 0