Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

style: address shellcheck warnings #1993

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

style: address shellcheck warnings #1993

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
fd5ca3f
use `$@` over `$*`. SC2048
reubenmiller Mar 13, 2024
a1e7a30
prevent globbing and word splitting. SC2086
reubenmiller Mar 13, 2024
99d56aa
read without -r will mangle backslashes. SC2162
reubenmiller Mar 13, 2024
bd01342
simplify by using `-n`. SC2236
reubenmiller Mar 13, 2024
739aa48
prevent shell from interpreting the -name value. SC2061
reubenmiller Mar 13, 2024
896ea2f
prevent globbing and word splitting. SC2086
reubenmiller Mar 13, 2024
9a18e6f
fixup! prevent globbing and word splitting. SC2086
reubenmiller May 14, 2024
31dc1e6
fixup! prevent globbing and word splitting. SC2086
reubenmiller May 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions bench/data/rsa-generate.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,20 @@ rm rsa-2048-65537.p8
m=(2048 3072 4096 8192)
for i in "${m[@]}"
do
echo $i
echo "$i"
openssl genpkey -algorithm RSA \
-pkeyopt rsa_keygen_bits:2048 \
-pkeyopt rsa_keygen_pubexp:3 | \
openssl pkcs8 -topk8 -nocrypt -outform der > rsa-$i-3.p8
openssl pkcs8 -topk8 -nocrypt -outform der > "rsa-$i-3.p8"

openssl pkey -pubout -inform der -outform der \
-in rsa-$i-3.p8 | \
-in "rsa-$i-3.p8" | \
openssl rsa -pubin -RSAPublicKey_out -inform DER -outform DER \
-out rsa-$i-3-public-key.der
-out "rsa-$i-3-public-key.der"

openssl dgst -sha256 -sign rsa-$i-3.p8 -out rsa-$i-3-signature.bin empty_message
openssl dgst -sha256 -sign "rsa-$i-3.p8" -out "rsa-$i-3-signature.bin" empty_message

rm rsa-$i-3.p8
rm "rsa-$i-3.p8"
done

rm empty_message
16 changes: 8 additions & 8 deletions mk/cargo.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ if [ -n "${ANDROID_NDK_ROOT-}" ]; then
android_tools=${ANDROID_NDK_ROOT}/toolchains/llvm/prebuilt/linux-x86_64/bin
fi

for arg in $*; do
for arg in "$@"; do
case $arg in
--target=*)
target=${arg#*=}
Expand Down Expand Up @@ -215,7 +215,7 @@ if [ -n "${RING_COVERAGE-}" ]; then
# something similar but different.
# export LLVM_PROFILE_FILE="$coverage_dir/%m.profraw"

target_upper=$(echo ${target_lower} | tr '[:lower:]' '[:upper:]')
target_upper=$(echo "${target_lower}" | tr '[:lower:]' '[:upper:]')

case "$OSTYPE" in
linux*)
Expand Down Expand Up @@ -251,16 +251,16 @@ cargo "$@"
if [ -n "${RING_COVERAGE-}" ]; then
# Keep in sync with check-symbol-prefixes.sh.
# Use the host target-libdir, not the target target-libdir.
llvm_root="$(rustc +${toolchain} --print target-libdir)/../bin"
llvm_root="$(rustc +"${toolchain}" --print target-libdir)/../bin"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks weird. I wonder if there's a better way of doing it. Maybe removing the "outermost"-looking double quotes, similar to the target_upper=... line above?

Copy link
Author

@reubenmiller reubenmiller May 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having outer quotes is fine because $() starts a new context (see SC2086) where the quoting is processed independently, so no escaping is required. However for consistency, I'll remove the outer quotes as it has not effect on the variable assignment.

Copy link
Author

@reubenmiller reubenmiller May 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved by 9a18e6f


while read executable; do
while read -r executable; do
basename=$(basename "$executable")
${llvm_root}/llvm-profdata merge -sparse "$coverage_dir/$basename.profraw" -o "$coverage_dir/$basename.profdata"
"${llvm_root}"/llvm-profdata merge -sparse "$coverage_dir/$basename.profraw" -o "$coverage_dir/$basename.profdata"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks weird to be inconsistent with having the double quotes only around the variable usage in "${llvm_root}"/llvm-profdata but then less tight in "$coverage_dir/$basename.profraw" and "$coverage_dir/$basename.profdata". Maybe I'm missing something, but some more consistency would be nice.

Copy link
Author

@reubenmiller reubenmiller May 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved by 31dc1e6.

Though I was thinking about normalizing the usage of ${example} vs $example using the following rules:

  • Use $example when the variable is not being without additional characters or variable modifiers (e.g. ${example:-default_value}

    value="$example"
if [ "$example" = "other" ]; then echo matches; fi

  • Use ${example} otherwise, e.g. +${example}, my/path/${example} to make it more (visually) obvious that a string is being built from mixing strings with variables.

    value="+${value}"
value2="${value:-default}"
"${bin}/path/myexec" run

@briansmith Are you ok with this?

mkdir -p "$coverage_dir"/reports
${llvm_root}/llvm-cov export \
--instr-profile "$coverage_dir"/$basename.profdata \
"${llvm_root}"/llvm-cov export \
--instr-profile "$coverage_dir/$basename.profdata" \
--format lcov \
"$executable" \
> "$coverage_dir"/reports/coverage-$basename.txt
> "$coverage_dir/reports/coverage-$basename.txt"
done < "$RING_BUILD_EXECUTABLE_LIST"
fi
8 changes: 4 additions & 4 deletions mk/check-symbol-prefixes.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
set -eux -o pipefail
IFS=$'\n\t'

for arg in $*; do
for arg in "$@"; do
case $arg in
--target=*)
target=${arg#*=}
Expand All @@ -32,7 +32,7 @@ done

# Keep in sync with cargo.sh.
# Use the host target-libdir, not the target target-libdir.
llvm_root="$(rustc +${toolchain} --print target-libdir)/../bin"
llvm_root="$(rustc +"${toolchain}" --print target-libdir)/../bin"

nm_exe="${llvm_root}/llvm-nm"

Expand All @@ -44,10 +44,10 @@ nm_exe="${llvm_root}/llvm-nm"
#
# This is very liberal in filtering out symbols that "look like"
# Rust-compiler-generated symbols.
find target/$target -type f -name libring-*.rlib | while read -r infile; do
find "target/$target" -type f -name "libring-*.rlib" | while read -r infile; do
bad=$($nm_exe --defined-only --extern-only --print-file-name "$infile" \
| ( grep -v -E " . _?(__imp__ZN4ring|ring_core_|__rustc|_ZN|DW.ref.rust_eh_personality)" || [[ $? == 1 ]] ))
if [ ! -z "${bad-}" ]; then
if [ -n "${bad-}" ]; then
echo "$bad"
exit 1
fi
Expand Down
2 changes: 1 addition & 1 deletion mk/clippy.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,4 @@ cargo clippy \
--deny missing_docs \
--deny unused_qualifications \
--deny warnings \
$NULL
"$NULL"
10 changes: 5 additions & 5 deletions mk/install-build-tools.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ set -eux -o pipefail
IFS=$'\n\t'

toolchain=stable
for arg in $*; do
for arg in "$@"; do
case $arg in
--target=*)
target=${arg#*=}
Expand Down Expand Up @@ -62,7 +62,7 @@ case ${target-} in

# XXX: Older Rust toolchain versions link with `-lgcc` instead of `-lunwind`;
# see https://github.com/rust-lang/rust/pull/85806.
find -L ${ANDROID_NDK_ROOT:-${ANDROID_HOME}/ndk/$ndk_version} -name libunwind.a \
find -L "${ANDROID_NDK_ROOT:-${ANDROID_HOME}/ndk/$ndk_version}" -name libunwind.a \
-execdir sh -c 'echo "INPUT(-lunwind)" > libgcc.a' \;
;;
esac
Expand Down Expand Up @@ -198,10 +198,10 @@ linux*)
;;
esac

rustup toolchain install --no-self-update --profile=minimal ${toolchain}
rustup toolchain install --no-self-update --profile=minimal "${toolchain}"
if [ -n "${target-}" ]; then
rustup target add --toolchain=${toolchain} ${target}
rustup target add --toolchain="${toolchain}" "${target}"
fi
if [ -n "${RING_COVERAGE-}" ]; then
rustup toolchain install --profile=minimal ${toolchain} --component llvm-tools-preview
rustup toolchain install --profile=minimal "${toolchain}" --component llvm-tools-preview
fi