All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
	Anthony PERARD <anthony.perard@vates.tech>,
	Michal Orzel <michal.orzel@amd.com>,
	Julien Grall <julien@xen.org>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Bertrand Marquis <bertrand.marquis@arm.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH 2/5] xen/tools: remove usages of `stat -s` in check-endbr.sh
Date: Wed, 11 Feb 2026 13:24:11 +0100	[thread overview]
Message-ID: <aYx064WIMMNmLAza@Mac.lan> (raw)
In-Reply-To: <35bdb10b-d39d-4e67-9ed8-c0b85ca5bd54@suse.com>

On Wed, Feb 11, 2026 at 12:40:58PM +0100, Jan Beulich wrote:
> On 11.02.2026 11:46, Roger Pau Monne wrote:
> > The `-s` option to stat is not POSIX compatible, and hence prevents the
> > check-endbr.sh script from running reliably.
> > 
> > The first instance of `stat -s` can be removed by fetching the section size
> > from the output of objdump itself, which the script already parses to get
> > the VMA values.
> 
> In both paragraphs, s/-s/-c/ ?

Bah, yes.

> > The other two instances can be replaced by counting the lines in the
> > respective files.  Those files contain list of addresses, so the size in
> > bytes is not strictly needed, we can count the number of lines instead.
> 
> Hmm, indeed, just that ...
> 
> > --- a/xen/tools/check-endbr.sh
> > +++ b/xen/tools/check-endbr.sh
> > @@ -92,14 +92,15 @@ ${OBJDUMP} -j .text $1 -d -w | grep '	endbr64 *$' | cut -f 1 -d ':' > $VALID &
> >  #    check nevertheless.
> >  #
> >  eval $(${OBJDUMP} -j .text $1 -h |
> > -    $AWK '$2 == ".text" {printf "vma_hi=%s\nvma_lo=%s\n", substr($4, 1, 9), substr($4, 10, 16)}')
> > +    $AWK '$2 == ".text" {printf "bin_sz=%s\nvma_hi=%s\nvma_lo=%s\n", "0x" $3, substr($4, 1, 9), substr($4, 10, 16)}')
> >  
> > -${OBJCOPY} -j .text $1 -O binary $TEXT_BIN
> > -
> > -bin_sz=$(stat -c '%s' $TEXT_BIN)
> > +# Convert objdump hex reported .text size to decimal
> > +bin_sz=$(printf %u $bin_sz)
> 
> (Alternatively without this line, but ...
> 
> >  [ "$bin_sz" -ge $(((1 << 28) - $vma_lo)) ] &&
> 
> [ "$(($bin_sz))" -ge $(((1 << 28) - $vma_lo)) ] &&
> 
> ?)

Isn't that bash-specific functionality?  The script interpreter is set
as /bin/sh.

> >      { echo "$MSG_PFX Error: .text offsets must not exceed 256M" >&2; exit 1; }
> >  
> > +${OBJCOPY} -j .text $1 -O binary $TEXT_BIN
> > +
> >  # instruction:    hex:           oct:
> >  # endbr64         f3 0f 1e fa    363 017 036 372
> >  # endbr32         f3 0f 1e fb    363 017 036 373
> > @@ -116,8 +117,8 @@ fi | $AWK -F':' '{printf "%s%07x\n", "'$vma_hi'", int('$((0x$vma_lo))') + $1}' >
> >  wait
> >  
> >  # Sanity check $VALID and $ALL, in case the string parsing bitrots
> > -val_sz=$(stat -c '%s' $VALID)
> > -all_sz=$(stat -c '%s' $ALL)
> > +val_sz=$(wc -l < $VALID)
> > +all_sz=$(wc -l < $ALL)
> >  [ "$val_sz" -eq 0 ]         && { echo "$MSG_PFX Error: Empty valid-addrs" >&2; exit 1; }
> >  [ "$all_sz" -eq 0 ]         && { echo "$MSG_PFX Error: Empty all-addrs" >&2; exit 1; }
> >  [ "$all_sz" -lt "$val_sz" ] && { echo "$MSG_PFX Error: More valid-addrs than all-addrs" >&2; exit 1; }
> 
> ... the variables' _sz suffixes then end up a little misleading. Not sure
> in how far we care. Perhaps not enough to warrant the bigger churn:
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> (with the description adjustment).

I could rename to _lines or some such, but I didn't see much benefit.
If you don't have a strong opinion towards renaming I will leave
as-is.

Thanks, Roger.


  reply	other threads:[~2026-02-11 12:24 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-11 10:46 [PATCH 0/5] darwin: expand cross-builds to also support x86 and testing Roger Pau Monne
2026-02-11 10:46 ` [PATCH 1/5] xen/x86: always consider '/' as a division in assembly Roger Pau Monne
2026-02-11 11:01   ` Bertrand Marquis
2026-02-11 11:11   ` Jan Beulich
2026-02-11 11:22     ` Roger Pau Monné
2026-02-11 10:46 ` [PATCH 2/5] xen/tools: remove usages of `stat -s` in check-endbr.sh Roger Pau Monne
2026-02-11 11:08   ` Bertrand Marquis
2026-02-11 11:40   ` Jan Beulich
2026-02-11 12:24     ` Roger Pau Monné [this message]
2026-02-11 13:07       ` Jan Beulich
2026-02-11 13:34         ` Roger Pau Monné
2026-02-11 13:43           ` Jan Beulich
2026-02-11 10:46 ` [PATCH 3/5] xen/tools: fix grep reporting 'illegal byte sequence' " Roger Pau Monne
2026-02-11 11:10   ` Bertrand Marquis
2026-02-11 10:46 ` [PATCH 4/5] cirrus-ci: add x86 and arm64 macOS hypervisor builds Roger Pau Monne
2026-02-11 10:46 ` [PATCH 5/5] cirrus-ci: add x86 XTF self-tests for macOS build Roger Pau Monne

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=aYx064WIMMNmLAza@Mac.lan \
    --to=roger.pau@citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=anthony.perard@vates.tech \
    --cc=bertrand.marquis@arm.com \
    --cc=jbeulich@suse.com \
    --cc=julien@xen.org \
    --cc=michal.orzel@amd.com \
    --cc=sstabellini@kernel.org \
    --cc=xen-devel@lists.xenproject.org \
    /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.