summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLuke Shumaker <lukeshu@parabola.nu>2016-04-17 12:18:02 -0400
committerEli Schwartz <eschwartz@archlinux.org>2018-03-19 23:43:46 -0400
commit23c2b82c336bf19b7a29a90d19bca4423d8b8839 (patch)
tree405adb9fea517ff93df2bd6569e3d3aafd47c062
parent97e17a5996425e8b7c3c1765a3c5074e1c4ff38d (diff)
Don't use `grep -q` when operating on piped stdin
(By default, prefer `grep &>/dev/null`) `grep -q` may exit as soon as it finds a match; this is a good optimization for when the input is a file. However, if the input is the output of another program, then that other program will receive SIGPIPE, and further writes will fail. When this happens, it might (bsdtar does) print a message about a "write error" to stderr. Which is going to confuse and alarm the user. In one of the cases (in common.bash, in the test suite), this had already been mitigated by wrapping bsdtar in "echo "$(bsdtar ...)", as Bash builtin echo doesn't complain if it gets SIGPIPE. However, that means we're storing the entire output of bsdtar in memory, which is silly. Additionally, the way it was implemented is also wrong; because it was being used with `grep -qv` instead of just `grep -q`, it *always* found a non-matching line (even something inconsequential like `%NAME%`), and *never* triggered a test failure. Looking at a few of these cases, it might also make sense to switch to using `bsdtar tf` instead of `bsdtar xf` when checking membership, but that's work for another day.
-rw-r--r--db-functions6
-rw-r--r--test/lib/common.bash6
2 files changed, 5 insertions, 7 deletions
diff --git a/db-functions b/db-functions
index 52faea2..4731f36 100644
--- a/db-functions
+++ b/db-functions
@@ -303,11 +303,7 @@ check_pkgfile() {
in_array "${pkgarch}" ${ARCHES[@]} 'any' || return 1
- if echo "${pkgfile##*/}" | grep -q "^${pkgname}-${pkgver}-${pkgarch}"; then
- return 0
- else
- return 1
- fi
+ [[ ${pkgfile##*/} = ${pkgname}-${pkgver}-${pkgarch}* ]]
}
check_pkgsvn() {
diff --git a/test/lib/common.bash b/test/lib/common.bash
index f9057f0..36c735f 100644
--- a/test/lib/common.bash
+++ b/test/lib/common.bash
@@ -226,7 +226,7 @@ checkPackageDB() {
for db in ${DBEXT} ${FILESEXT}; do
[ -r "${FTP_BASE}/${repo}/os/${repoarch}/${repo}${db%.tar.*}" ]
- bsdtar -xf "${FTP_BASE}/${repo}/os/${repoarch}/${repo}${db%.tar.*}" -O | grep -q "${pkgfile%${PKGEXT}}"
+ bsdtar -xf "${FTP_BASE}/${repo}/os/${repoarch}/${repo}${db%.tar.*}" -O | grep "${pkgfile%${PKGEXT}}" &>/dev/null
done
done
done
@@ -280,7 +280,9 @@ checkRemovedPackageDB() {
for tarch in ${tarches[@]}; do
if [ -r "${FTP_BASE}/${repo}/os/${tarch}/${repo}${db%.tar.*}" ]; then
for pkgname in ${pkgnames[@]}; do
- echo "$(bsdtar -xf "${FTP_BASE}/${repo}/os/${tarch}/${repo}${db%.tar.*}" -O)" | grep -qv ${pkgname}
+ if bsdtar -xf "${FTP_BASE}/${repo}/os/${tarch}/${repo}${db%.tar.*}" -O | grep ${pkgname} &>/dev/null; then
+ return 1
+ fi
done
fi
done