Re: [PATCH 7/7] perf build: install-build-deps: add RHEL family devel package mapping

[email protected]
Newsgroups org.kernel.vger.linux-perf-users
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The INT and TERM signal trap removes the temporary file but fails to exit the script, causing the script to resume execution and proceed with an empty or partial package list after an interrupt.
--

--- Patch [7]: [PATCH 7/7] perf build: install-build-deps: add RHEL family devel package mapping ---
commit d7066a2ea105c214aa5807a82fc683f385da44a7
Author: Arnaldo Carvalho de Melo <[email protected]>

perf build: install-build-deps: add RHEL family devel package mapping

This patch extends the Fedora package mapping in install-build-deps.sh to the
RHEL family (RHEL, CentOS Stream, Rocky Linux, AlmaLinux, Oracle Linux). It
adds logic to check enabled repositories via dnf repoquery to handle packages
that are missing or located in optional repositories like CRB or EPEL.

> diff --git a/tools/perf/scripts/install-build-deps.sh b/tools/perf/scripts/install-build-deps.sh
> index 4a6f4bda49094..caf6d8d1e56d1 100755
> --- a/tools/perf/scripts/install-build-deps.sh
> +++ b/tools/perf/scripts/install-build-deps.sh
[ ... ]
> @@ -518,23 +518,43 @@ main() {
[ ... ]
> +	missing_file=$(mktemp) || {
> +		echo "error: cannot create a temporary file (mktemp failed)" >&2
> +		exit 1
> +	}
> +	# package_set() runs in a subshell, doing dnf repoquery loops that
> +	# may be interrupted, so also trap the temp file then.
> +	trap 'rm -f "$missing_file"' EXIT INT TERM
> +	pkgs=$(package_set "$distro" "$srcdir" 2>"$missing_file")
> +	missing_pkgs=$(cat "$missing_file")

[Severity: Medium]
Will this script continue execution if the user sends SIGINT?

Since the trap removes the temporary file but doesn't explicitly exit, an
interrupt during package_set() would delete the file and then resume execution.
The cat command would fail because the file is gone, and the script might
continue on to the sudo installation phase with an incomplete package list.

Should the trap call exit for INT and TERM signals?

> +	rm -f "$missing_file"
>  
>  	case "$action" in
>  	list)
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.