diff options
author | Luke Shumaker <lukeshu@parabola.nu> | 2016-04-17 12:18:02 -0400 |
---|---|---|
committer | Eli Schwartz <eschwartz@archlinux.org> | 2018-03-19 23:43:46 -0400 |
commit | 23c2b82c336bf19b7a29a90d19bca4423d8b8839 (patch) | |
tree | 405adb9fea517ff93df2bd6569e3d3aafd47c062 | |
parent | 97e17a5996425e8b7c3c1765a3c5074e1c4ff38d (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-functions | 6 | ||||
-rw-r--r-- | test/lib/common.bash | 6 |
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 |