All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Yann E. MORIN" <yann.morin.1998@free.fr>
To: Florian Fainelli <f.fainelli@gmail.com>
Cc: buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH] package/linux-tools: Exclude checking PE binaries from perf test
Date: Mon, 17 Apr 2023 21:58:16 +0200	[thread overview]
Message-ID: <20230417195816.GN2819@scaer> (raw)
In-Reply-To: <20220927221133.594071-1-f.fainelli@gmail.com>

Florian, All,

On 2022-09-27 15:11 -0700, Florian Fainelli spake thusly:
> Since upstream Linux commit ed21d6d7c48e6e96c2d617e304a7ebfbd17b1807
> ("perf tests: Add test for PE binary format support") present in >=
> v5.10 there is an unconditional installation of PE binaries which will
> be rejected by the check-bin-arch script.
> 
> Make sure that these binaries are excluded from being checked to allow
> the installation of the perf tests.
> 
> Fixes: 6fcdaa4c5096 ("package/linux-tools: Allow installation of perf scripts")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

I've finally applied to master, thanks.

However, I wonder if we could not be a bit more robust in check-bin-arch
itself, something like:

    diff --git a/support/scripts/check-bin-arch b/support/scripts/check-bin-arch
    index 27cc59bca0..2920a95ae3 100755
    --- a/support/scripts/check-bin-arch
    +++ b/support/scripts/check-bin-arch
    @@ -58,6 +58,9 @@ exitcode=0
     IFS="
     "
     
    +# ELF magic
    +ELF_MAGIC="$(printf "\x7f\x45\x4c\x46")"
    +
     while read f; do
     	for ignore in "${IGNORES[@]}"; do
     		if [[ "${f}" =~ ^"${ignore}" ]]; then
    @@ -71,13 +74,19 @@ while read f; do
     		continue
     	fi
     
    +	# Check if it looks like it is an ELF file
    +	read -N 4 magic <"${TARGET_DIR}/${f}"
    +	if [ "${magic}" != "${ELF_MAGIC}" ]; then
    +		continue
    +	fi
    +
     	# Get architecture using readelf. We pipe through 'head -1' so
     	# that when the file is a static library (.a), we only take
     	# into account the architecture of the first object file.
     	arch=$(LC_ALL=C ${readelf} -h "${TARGET_DIR}/${f}" 2>&1 | \
     		       sed -r -e '/^  Machine: +(.+)/!d; s//\1/;' | head -1)
     
    -# If no architecture found, assume it was not an ELF file
    +# If no architecture found, assume it was in fact not an ELF file
     	if test "${arch}" = "" ; then
     		continue
     	fi

(with the read -N 4 trick as discussed in [0])

Thoughts?

[0] https://lore.kernel.org/buildroot/10267_1663749711_632ACE4F_10267_83_8_20220921084149.GB2398274@tl-lnx-nyma7486/

Regards,
Yann E. MORIN.

> ---
>  package/linux-tools/linux-tool-perf.mk.in | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/package/linux-tools/linux-tool-perf.mk.in b/package/linux-tools/linux-tool-perf.mk.in
> index dda63cccecb4..c22097316264 100644
> --- a/package/linux-tools/linux-tool-perf.mk.in
> +++ b/package/linux-tools/linux-tool-perf.mk.in
> @@ -169,6 +169,10 @@ define PERF_INSTALL_REMOVE_SCRIPTS
>  	$(RM) -rf $(TARGET_DIR)/usr/libexec/perf-core/scripts/
>  	$(RM) -rf $(TARGET_DIR)/usr/libexec/perf-core/tests/
>  endef
> +
> +LINUX_TOOLS_BIN_ARCH_EXCLUDE += \
> +	/usr/libexec/perf-core/tests/pe-file.exe \
> +	/usr/libexec/perf-core/tests/pe-file.exe.debug
>  endif
>  
>  define PERF_INSTALL_TARGET_CMDS
> -- 
> 2.25.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

  parent reply	other threads:[~2023-04-17 19:58 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-27 22:11 [Buildroot] [PATCH] package/linux-tools: Exclude checking PE binaries from perf test Florian Fainelli
2022-09-28 21:38 ` Thomas Petazzoni
2022-09-28 22:28   ` Florian Fainelli
2022-09-29  6:40     ` Thomas Petazzoni
2022-09-30 22:05       ` Florian Fainelli
2022-10-01 19:07     ` Yann E. MORIN
2022-10-03 17:21       ` Florian Fainelli
2022-10-03 19:15         ` Yann E. MORIN
2023-04-17 19:58 ` Yann E. MORIN [this message]
2023-04-19  0:27   ` Florian Fainelli
2023-04-23 10:40 ` Peter Korsgaard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230417195816.GN2819@scaer \
    --to=yann.morin.1998@free.fr \
    --cc=buildroot@buildroot.org \
    --cc=f.fainelli@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.