Re: [PATCH 2/5] xen/tools: remove usages of `stat -s` in check-endbr.sh

2026-02-11 Thread Jan Beulich
On 11.02.2026 14:34, Roger Pau Monné wrote:
> On Wed, Feb 11, 2026 at 02:07:13PM +0100, Jan Beulich wrote:
>> On 11.02.2026 13:24, Roger Pau Monné wrote:
>>> On Wed, Feb 11, 2026 at 12:40:58PM +0100, Jan Beulich wrote:
 On 11.02.2026 11:46, Roger Pau Monne wrote:
> --- 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.
>>
>> I, too, would have thought so, but then the rhs of the -ge already used 
>> $((...)).
> 
> OK, yes, I didn't realize those existing usages.  Now that I look at
> it it does seem to have a bunch of other bashisms, for example
> parameter expansion.

Then perhaps in another patch we want to properly mark it as a bash script.

Jan



Re: [PATCH 2/5] xen/tools: remove usages of `stat -s` in check-endbr.sh

2026-02-11 Thread Roger Pau Monné
On Wed, Feb 11, 2026 at 02:07:13PM +0100, Jan Beulich wrote:
> On 11.02.2026 13:24, Roger Pau Monné wrote:
> > On Wed, Feb 11, 2026 at 12:40:58PM +0100, Jan Beulich wrote:
> >> On 11.02.2026 11:46, Roger Pau Monne wrote:
> >>> --- 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.
> 
> I, too, would have thought so, but then the rhs of the -ge already used 
> $((...)).

OK, yes, I didn't realize those existing usages.  Now that I look at
it it does seem to have a bunch of other bashisms, for example
parameter expansion.

I will adjust that plus the commit message and take your RB.

Thanks, Roger.



Re: [PATCH 2/5] xen/tools: remove usages of `stat -s` in check-endbr.sh

2026-02-11 Thread Jan Beulich
On 11.02.2026 13:24, Roger Pau Monné wrote:
> On Wed, Feb 11, 2026 at 12:40:58PM +0100, Jan Beulich wrote:
>> On 11.02.2026 11:46, Roger Pau Monne wrote:
>>> --- 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.

I, too, would have thought so, but then the rhs of the -ge already used 
$((...)).

Jan



Re: [PATCH 2/5] xen/tools: remove usages of `stat -s` in check-endbr.sh

2026-02-11 Thread Roger Pau Monné
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 fa363 017 036 372
> >  # endbr32 f3 0f 1e fb363 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 
> (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.



Re: [PATCH 2/5] xen/tools: remove usages of `stat -s` in check-endbr.sh

2026-02-11 Thread Jan Beulich
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/ ?

> 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)) ] &&

?)

>  { 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 fa363 017 036 372
>  # endbr32 f3 0f 1e fb363 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 
(with the description adjustment).

Jan



Re: [PATCH 2/5] xen/tools: remove usages of `stat -s` in check-endbr.sh

2026-02-11 Thread Bertrand Marquis
Hi Roger,

> On 11 Feb 2026, at 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.
> 
> 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.

I can confirm compilation works on my side and changes seem coherent
but i would like an x86 maintainer to review/ack this one to confirm that
functionally it is ok.

> 
> Suggested-by: Bertrand Marquis 
> Signed-off-by: Roger Pau Monné 

Acked-by: Bertrand Marquis 

Cheers
Bertrand

> ---
> xen/tools/check-endbr.sh | 13 +++--
> 1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/tools/check-endbr.sh b/xen/tools/check-endbr.sh
> index bf153a570db4..383d7e710a53 100755
> --- 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)
> [ "$bin_sz" -ge $(((1 << 28) - $vma_lo)) ] &&
> { 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 fa363 017 036 372
> # endbr32 f3 0f 1e fb363 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; }
> -- 
> 2.51.0
>