Skip to content

tests/containers/fips-nginx: fix typos and add trap in build.sh#4492

Open
HuijingHei wants to merge 1 commit intocoreos:mainfrom
HuijingHei:fix-typos
Open

tests/containers/fips-nginx: fix typos and add trap in build.sh#4492
HuijingHei wants to merge 1 commit intocoreos:mainfrom
HuijingHei:fix-typos

Conversation

@HuijingHei
Copy link
Member

Fixes #4477

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request fixes a typo in the fips-nginx README and improves the build.sh script by adding a trap to ensure cleanup of the temporary directory. I've suggested a small improvement to make the trap more robust.


tmpdir="$(mktemp -d)"
cp Containerfile ${tmpdir}
trap 'rm -rf "${tmpdir}"' EXIT
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

For better robustness, it's a good practice to change out of the directory that is about to be removed. While rm -rf on the current working directory often works, it's not guaranteed to be portable or successful in all cases (e.g., on certain filesystems or configurations). Changing to the home directory with cd before the rm command ensures the cleanup is more reliable.

Suggested change
trap 'rm -rf "${tmpdir}"' EXIT
trap 'cd && rm -rf "${tmpdir}"' EXIT

@HuijingHei
Copy link
Member Author

Next time I think better to let gemini review again before merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant