[pacman-dev] [PATCH 1/3] Store a package info level flag if we fail to load data
If we are missing a local database file, we get repeated messages over and over telling us the same thing, rather than being sane and erroring only once. This package adds an INFRQ_ERROR level that is added to the mask if we encounter any errors on a local_db_read() operation, and short circuits future calls if found in the value. This fixes FS#25313. Note that this does not make any behavior changes other than suppressing error messages and repeated code calls to failure cases; we still have more to do in the "local database is hosed" department. Also make a small update to the wrong but unused flags set in be_package; using INFRQ_ALL there was not totally correct. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/be_local.c | 14 +++++++++++--- lib/libalpm/be_package.c | 4 ++-- lib/libalpm/db.h | 3 ++- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c index 261ad87..73b533f 100644 --- a/lib/libalpm/be_local.c +++ b/lib/libalpm/be_local.c @@ -533,6 +533,13 @@ static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq) /* already loaded all of this info, do nothing */ return 0; } + + if(info->infolevel & INFRQ_ERROR) { + /* We've encountered an error loading this package before. Don't attempt + * repeated reloads, just give up. */ + return -1; + } + _alpm_log(db->handle, ALPM_LOG_FUNCTION, "loading package data for %s : level=0x%x\n", info->name, inforeq); @@ -619,6 +626,7 @@ static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq) } fclose(fp); fp = NULL; + info->infolevel |= INFRQ_DESC; } /* FILES */ @@ -673,6 +681,7 @@ static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq) } fclose(fp); fp = NULL; + info->infolevel |= INFRQ_FILES; } /* INSTALL */ @@ -681,15 +690,14 @@ static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq) if(access(path, F_OK) == 0) { info->scriptlet = 1; } + info->infolevel |= INFRQ_SCRIPTLET; } - /* internal */ - info->infolevel |= inforeq; - free(pkgpath); return 0; error: + info->infolevel |= INFRQ_ERROR; free(pkgpath); if(fp) { fclose(fp); diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index 0edaa5a..550f303 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -437,9 +437,9 @@ alpm_pkg_t *_alpm_pkg_load_internal(alpm_handle_t *handle, const char *pkgfile, _alpm_log(handle, ALPM_LOG_DEBUG, "sorting package filelist for %s\n", pkgfile); newpkg->files.files = files_msort(files, files_count); newpkg->files.count = files_count; - newpkg->infolevel = INFRQ_ALL; + newpkg->infolevel = INFRQ_BASE | INFRQ_DESC | INFRQ_FILES | INFRQ_SCRIPTLET; } else { - newpkg->infolevel = INFRQ_BASE | INFRQ_DESC; + newpkg->infolevel = INFRQ_BASE | INFRQ_DESC | INFRQ_SCRIPTLET; } return newpkg; diff --git a/lib/libalpm/db.h b/lib/libalpm/db.h index 5c7988b..a09c7e2 100644 --- a/lib/libalpm/db.h +++ b/lib/libalpm/db.h @@ -40,7 +40,8 @@ typedef enum _alpm_dbinfrq_t { INFRQ_SCRIPTLET = (1 << 3), INFRQ_DSIZE = (1 << 4), /* ALL should be info stored in the package or database */ - INFRQ_ALL = 0x1F + INFRQ_ALL = 0x1F, + INFRQ_ERROR = (1 << 31) } alpm_dbinfrq_t; /** Database status. Bitflags. */ -- 1.7.6
This is more in line with standard Python practice, and makes keyboard interrupts behave a lot more sanely. It also prevents the useless spawning of a shell as well as simplifies the command building and working directory stuff. Signed-off-by: Dan McGee <dan@archlinux.org> --- test/pacman/pmtest.py | 46 +++++++++++++++++++++++----------------------- 1 files changed, 23 insertions(+), 23 deletions(-) diff --git a/test/pacman/pmtest.py b/test/pacman/pmtest.py index e38d3a6..0572d6f 100644 --- a/test/pacman/pmtest.py +++ b/test/pacman/pmtest.py @@ -17,8 +17,10 @@ import os +import shlex import shutil import stat +import subprocess import time import pmrule @@ -181,7 +183,7 @@ def run(self, pacman): print "==> Running test" vprint("\tpacman %s" % self.args) - cmd = [""] + cmd = [] if os.geteuid() != 0: fakeroot = util.which("fakeroot") if not fakeroot: @@ -194,41 +196,39 @@ def run(self, pacman): cmd.append("fakechroot") if pacman["gdb"]: - cmd.append("libtool execute gdb --args") + cmd.extend(["libtool", "execute", "gdb", "--args"]) if pacman["valgrind"]: - cmd.append("valgrind -q --tool=memcheck --leak-check=full --show-reachable=yes --suppressions=%s/valgrind.supp" % os.getcwd()) - cmd.append("\"%s\" --config=\"%s\" --root=\"%s\" --dbpath=\"%s\" --cachedir=\"%s\"" \ - % (pacman["bin"], - os.path.join(self.root, util.PACCONF), - self.root, - os.path.join(self.root, util.PM_DBPATH), - os.path.join(self.root, util.PM_CACHEDIR))) + cmd.extend(["libtool", "execute", "valgrind", "-q", + "--tool=memcheck", "--leak-check=full", + "--show-reachable=yes", "--suppressions=%s/valgrind.supp" % os.getcwd()]) + cmd.extend([pacman["bin"], + "--config", os.path.join(self.root, util.PACCONF), + "--root", self.root, + "--dbpath", os.path.join(self.root, util.PM_DBPATH), + "--cachedir", os.path.join(self.root, util.PM_CACHEDIR)]) if not pacman["manual-confirm"]: cmd.append("--noconfirm") if pacman["debug"]: cmd.append("--debug=%s" % pacman["debug"]) - cmd.append("%s" % self.args) - if not pacman["gdb"] and not pacman["valgrind"] and not pacman["nolog"]: - cmd.append(">\"%s\" 2>&1" % os.path.join(self.root, util.LOGFILE)) + cmd.extend(shlex.split(self.args)) + if not (pacman["gdb"] or pacman["valgrind"] or pacman["nolog"]): + output = open(os.path.join(self.root, util.LOGFILE), 'w') + else: + output = None vprint("\trunning: %s" % " ".join(cmd)) # Change to the tmp dir before running pacman, so that local package # archives are made available more easily. - curdir = os.getcwd() - tmpdir = os.path.join(self.root, util.TMPDIR) - os.chdir(tmpdir) - time_start = time.time() - self.retcode = os.system(" ".join(cmd)) + self.retcode = subprocess.call(cmd, stdout=output, stderr=output, + cwd=os.path.join(self.root, util.TMPDIR)) time_end = time.time() - vprint("\ttime elapsed: %ds" % (time_end - time_start)) + vprint("\ttime elapsed: %.2fs" % (time_end - time_start)) + + if output: + output.close() - if self.retcode == None: - self.retcode = 0 - else: - self.retcode /= 256 vprint("\tretcode = %s" % self.retcode) - os.chdir(curdir) # Check if the lock is still there if os.path.isfile(util.PM_LOCK): -- 1.7.6
We don't write with extra or unknown whitespace, so there is little reason for us to trim it when reading either. This also fixes the hopefully never encountered "paths that start or end with spaces" issue, for which two pactests have been added. The tests also contain other evil characters that we have encountered before and handle just fine, but it doesn't hurt to ensure we don't break such support in the future. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/be_local.c | 12 ++++---- lib/libalpm/be_package.c | 7 ++--- lib/libalpm/be_sync.c | 12 +++++---- lib/libalpm/util.c | 20 +++++++++++++++ lib/libalpm/util.h | 1 + test/pacman/pmrule.py | 2 +- test/pacman/tests/remove071.py | 33 +++++++++++++++++++++++++ test/pacman/tests/sync600.py | 51 ++++++++++++++++++++++++++++++++++++++++ 8 files changed, 122 insertions(+), 16 deletions(-) create mode 100644 test/pacman/tests/remove071.py create mode 100644 test/pacman/tests/sync600.py diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c index 73b533f..5d136c9 100644 --- a/lib/libalpm/be_local.c +++ b/lib/libalpm/be_local.c @@ -500,7 +500,7 @@ static char *get_pkgpath(alpm_db_t *db, alpm_pkg_t *info) #define READ_NEXT() do { \ if(fgets(line, sizeof(line), fp) == NULL && !feof(fp)) goto error; \ - _alpm_strtrim(line); \ + _alpm_strip_newline(line); \ } while(0) #define READ_AND_STORE(f) do { \ @@ -510,8 +510,8 @@ static char *get_pkgpath(alpm_db_t *db, alpm_pkg_t *info) #define READ_AND_STORE_ALL(f) do { \ char *linedup; \ - READ_NEXT(); \ - if(strlen(line) == 0) break; \ + if(fgets(line, sizeof(line), fp) == NULL && !feof(fp)) goto error; \ + if(_alpm_strip_newline(line) == 0) break; \ STRDUP(linedup, line, goto error); \ f = alpm_list_add(f, linedup); \ } while(1) /* note the while(1) and not (0) */ @@ -637,12 +637,12 @@ static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq) goto error; } while(fgets(line, sizeof(line), fp)) { - _alpm_strtrim(line); + _alpm_strip_newline(line); if(strcmp(line, "%FILES%") == 0) { size_t files_count = 0, files_size = 0; alpm_file_t *files = NULL; - while(fgets(line, sizeof(line), fp) && strlen(_alpm_strtrim(line))) { + while(fgets(line, sizeof(line), fp) && _alpm_strip_newline(line)) { if(files_count >= files_size) { size_t old_size = files_size; if(files_size == 0) { @@ -669,7 +669,7 @@ static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq) info->files.count = files_count; info->files.files = files; } else if(strcmp(line, "%BACKUP%") == 0) { - while(fgets(line, sizeof(line), fp) && strlen(_alpm_strtrim(line))) { + while(fgets(line, sizeof(line), fp) && _alpm_strip_newline(line)) { alpm_backup_t *backup; CALLOC(backup, 1, sizeof(alpm_backup_t), goto error); if(_alpm_split_backup(line, &backup)) { diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index 550f303..f80790c 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -149,13 +149,13 @@ static int parse_descfile(alpm_handle_t *handle, struct archive *a, alpm_pkg_t * /* loop until we reach EOF or other error */ while((ret = _alpm_archive_fgets(a, &buf)) == ARCHIVE_OK) { - char *line = _alpm_strtrim(buf.line); + size_t len = _alpm_strip_newline(buf.line); linenum++; - if(strlen(line) == 0 || line[0] == '#') { + if(len == 0 || buf.line[0] == '#') { continue; } - ptr = line; + ptr = buf.line; key = strsep(&ptr, "="); if(key == NULL || ptr == NULL) { _alpm_log(handle, ALPM_LOG_DEBUG, "%s: syntax error in description file line %d\n", @@ -213,7 +213,6 @@ static int parse_descfile(alpm_handle_t *handle, struct archive *a, alpm_pkg_t * newpkg->name ? newpkg->name : "error", key, linenum); } } - line[0] = '\0'; } if(ret != ARCHIVE_EOF) { _alpm_log(handle, ALPM_LOG_DEBUG, "error parsing package descfile\n"); diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c index 07356f0..d4c71a8 100644 --- a/lib/libalpm/be_sync.c +++ b/lib/libalpm/be_sync.c @@ -443,7 +443,8 @@ static int sync_db_populate(alpm_db_t *db) #define READ_NEXT() do { \ if(_alpm_archive_fgets(archive, &buf) != ARCHIVE_OK) goto error; \ - line = _alpm_strtrim(buf.line); \ + line = buf.line; \ + _alpm_strip_newline(line); \ } while(0) #define READ_AND_STORE(f) do { \ @@ -453,9 +454,9 @@ static int sync_db_populate(alpm_db_t *db) #define READ_AND_STORE_ALL(f) do { \ char *linedup; \ - READ_NEXT(); \ - if(strlen(line) == 0) break; \ - STRDUP(linedup, line, goto error); \ + if(_alpm_archive_fgets(archive, &buf) != ARCHIVE_OK) goto error; \ + if(_alpm_strip_newline(buf.line) == 0) break; \ + STRDUP(linedup, buf.line, goto error); \ f = alpm_list_add(f, linedup); \ } while(1) /* note the while(1) and not (0) */ @@ -493,7 +494,8 @@ static int sync_db_read(alpm_db_t *db, struct archive *archive, || strcmp(filename, "deltas") == 0) { int ret; while((ret = _alpm_archive_fgets(archive, &buf)) == ARCHIVE_OK) { - char *line = _alpm_strtrim(buf.line); + char *line = buf.line; + _alpm_strip_newline(line); if(strcmp(line, "%NAME%") == 0) { READ_NEXT(); diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index 030cf43..123cd24 100644 --- a/lib/libalpm/util.c +++ b/lib/libalpm/util.c @@ -207,6 +207,26 @@ char *_alpm_strtrim(char *str) return str; } +/** + * Trim trailing newline from a string (if one exists). + * @param str a single line of text + * @return the length of the trimmed string + */ +size_t _alpm_strip_newline(char *str) +{ + size_t len; + if(str == '\0') { + return 0; + } + len = strlen(str); + while(str[len - 1] == '\n') { + len--; + } + str[len] = '\0'; + + return len; +} + /* Compression functions */ /** diff --git a/lib/libalpm/util.h b/lib/libalpm/util.h index a75c5aa..9ee6370 100644 --- a/lib/libalpm/util.h +++ b/lib/libalpm/util.h @@ -95,6 +95,7 @@ int _alpm_makepath(const char *path); int _alpm_makepath_mode(const char *path, mode_t mode); int _alpm_copyfile(const char *src, const char *dest); char *_alpm_strtrim(char *str); +size_t _alpm_strip_newline(char *str); int _alpm_unpack_single(alpm_handle_t *handle, const char *archive, const char *prefix, const char *filename); int _alpm_unpack(alpm_handle_t *handle, const char *archive, const char *prefix, diff --git a/test/pacman/pmrule.py b/test/pacman/pmrule.py index cb7ae88..d64f519 100644 --- a/test/pacman/pmrule.py +++ b/test/pacman/pmrule.py @@ -42,7 +42,7 @@ def check(self, test): [testname, args] = self.rule.split("=") if testname[0] == "!": self.false = 1 - testname = testname.lstrip("!") + testname = testname[1:] [kind, case] = testname.split("_") if "|" in args: [key, value] = args.split("|", 1) diff --git a/test/pacman/tests/remove071.py b/test/pacman/tests/remove071.py new file mode 100644 index 0000000..eff70a4 --- /dev/null +++ b/test/pacman/tests/remove071.py @@ -0,0 +1,33 @@ +# coding=utf8 +self.description = "Remove packages with evil filenames" + +self.filesystem = ["usr/bin/endwithspace", + "spaces/name"] + +p1 = pmpkg("spaces") +p1.files = ["usr/bin/endwithspace ", + " spaces/name"] +self.addpkg2db("local", p1) + +p2 = pmpkg("unicodechars") +# somewhat derived from FS#9906 +p2.files = ["usr/share/Märchen", + "usr/share/ƏƐƕƺ", + "usr/share/предупреждение", + "usr/share/סֶאבױ", + "usr/share/←↯↻⇈", + "usr/share/アヅヨヾ", + "usr/share/错误"] +self.addpkg2db("local", p2) + +self.args = "-R %s %s" % (p1.name, p2.name) + +self.addrule("PACMAN_RETCODE=0") +self.addrule("!PKG_EXIST=%s" % p1.name) +self.addrule("!PKG_EXIST=%s" % p2.name) + +for f in p1.files: + self.addrule("!FILE_EXIST=%s" % f) + self.addrule("FILE_EXIST=%s" % f.strip()) +for f in p2.files: + self.addrule("!FILE_EXIST=%s" % f) diff --git a/test/pacman/tests/sync600.py b/test/pacman/tests/sync600.py new file mode 100644 index 0000000..e0be668 --- /dev/null +++ b/test/pacman/tests/sync600.py @@ -0,0 +1,51 @@ +# coding=utf8 +self.description = "Sync packages with evil filenames" + +self.filesystem = ["usr/bin/endwithspace", + "usr/bin/newendwithspace", + "usr/bin/disappear", + "spaces/name", + "spaces/name2"] + +p1 = pmpkg("spaces") +p1.files = ["usr/bin/endwithspace ", + "usr/bin/disappear ", + " spaces/name", + " spaces/gone"] +self.addpkg2db("local", p1) + +sp1 = pmpkg("spaces", "1.1-1") +sp1.files = ["usr/bin/endwithspace ", + "usr/bin/newendwithspace ", + " spaces/name", + " spaces/name2"] +self.addpkg2db("sync", sp1) + +names = ["Märchen", "ƏƐƕƺ", "предупреждение", "סֶאבױ", + "←↯↻⇈", "アヅヨヾ", "错误"] + +p2 = pmpkg("unicodechars") +# somewhat derived from FS#9906 +p2.files = ["usr/share/%s" % name for name in names] +self.addpkg2db("local", p2) + +sp2 = pmpkg("unicodechars", "2.0-1") +sp2.files = ["usr/man/%s" % name for name in names] +self.addpkg2db("sync", sp2) + +self.args = "-S %s %s" % (sp1.name, sp2.name) + +self.addrule("PACMAN_RETCODE=0") +self.addrule("PKG_VERSION=%s|%s" % (sp1.name, sp1.version)) +self.addrule("PKG_VERSION=%s|%s" % (sp2.name, sp2.version)) + +for f in self.filesystem: + self.addrule("FILE_EXIST=%s" % f) +self.addrule("FILE_EXIST=usr/bin/endwithspace ") +self.addrule("FILE_EXIST= spaces/name") +self.addrule("FILE_EXIST= spaces/name2") +self.addrule("!FILE_EXIST=usr/bin/disappear ") +for f in p2.files: + self.addrule("!FILE_EXIST=%s" % f) +for f in sp2.files: + self.addrule("FILE_EXIST=%s" % f) -- 1.7.6
participants (1)
-
Dan McGee