[PATCH 0/2] livepatch-build-tools: fixes for non GNU tools

[PATCH 0/2] livepatch-build-tools: fixes for non GNU tools

[Roger Pau Monne]

Hello,

A couple more fixes to allow the usage of livepatch-build-tools with non
GNU utilities.  Shouldn't introduce any functional change.

Thanks, Roger.

Roger Pau Monne (2):
  livepatch-build-tools: remove usage of gawk
  livepatch-build-tools: do not use readlink -m option

 livepatch-build | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)


base-commit: e588b7914e7afa3abb64b15a32fc2fdb57ded341
-- 
2.43.0

[PATCH 1/2] livepatch-build-tools: remove usage of gawk

[Roger Pau Monne]

And instead use plain awk.

There's no need to use the --non-decimal-data option for gawk, since the
numbers that we want to print are already prefixed with '0x', and so plain awk
will do the conversion from hexadecimal to decimal just fine.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 livepatch-build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/livepatch-build b/livepatch-build
index 91d203bda0eb..26c88a2ed2c3 100755
--- a/livepatch-build
+++ b/livepatch-build
@@ -422,7 +422,7 @@ if [ "${SKIP}" != "build" ]; then
     echo "Reading special section data"
     # Using xen-syms built in the previous step by build_full().
     SPECIAL_VARS=$(readelf -wi "$OUTPUT/xen-syms" |
-               gawk --non-decimal-data '
+               awk '
                BEGIN { a = b = e = 0 }
                a == 0 && /DW_AT_name.* alt_instr/ {a = 1; next}
                b == 0 && /DW_AT_name.* bug_frame/ {b = 1; next}

base-commit: e588b7914e7afa3abb64b15a32fc2fdb57ded341
-- 
2.43.0

Re: [PATCH 1/2] livepatch-build-tools: remove usage of gawk

[Ross Lagerwall]

On Thu, Nov 23, 2023 at 4:08 PM Roger Pau Monne <roger.pau@citrix.com> wrote:
>
> And instead use plain awk.
>
> There's no need to use the --non-decimal-data option for gawk, since the
> numbers that we want to print are already prefixed with '0x', and so plain awk
> will do the conversion from hexadecimal to decimal just fine.

I don't think that's true (at least with gnu awk 5.1.1):

$ echo '<e98b7>   DW_AT_byte_size   : 0x14' | awk '{printf("%d\n", $4)}'
0

Having said that, my version of readelf actually gives the value in
decimal so the patch still works. But is that the case for all
versions of readelf? I assume the code was written like that for a
reason...

Ross

Re: [PATCH 1/2] livepatch-build-tools: remove usage of gawk

[Alejandro Vallejo]

On 24/11/2023 11:43, Ross Lagerwall wrote:
 > On Thu, Nov 23, 2023 at 4:08 PM Roger Pau Monne 
<roger.pau@citrix.com> wrote:
 >>
 >> And instead use plain awk.
 >>
 >> There's no need to use the --non-decimal-data option for gawk, since the
 >> numbers that we want to print are already prefixed with '0x', and so 
plain awk
 >> will do the conversion from hexadecimal to decimal just fine.
 >
 > I don't think that's true (at least with gnu awk 5.1.1):
 >
 > $ echo '<e98b7>   DW_AT_byte_size   : 0x14' | awk '{printf("%d\n", $4)}'
 > 0
 >

I think it's a FreeBSD quirk. From their man page:

 >        Historically,  awk did not accept "0x" as a hex string. 
However, since
 >        One True Awk used strtod to convert strings to floats, and 
since "0x12"
 >        is a valid hexadecimal representation of a floating  point 
number,  On
 >        FreeBSD, awk has accepted this notation as an extension since 
One True
 >        Awk was imported in FreeBSD 5.0. Upstream One True  Awk has 
restored
 >        the  historical behavior for better compatibility between the 
different
 >        awk implementations.  Both gawk and mawk already  behave 
similarly.
 >        Starting with FreeBSD 14.0 awk will no longer accept this 
extension.

I'm guessing the latest FreeBSD doesn't have that behaviour either?

Cheers,
Alejandro

Re: [PATCH 1/2] livepatch-build-tools: remove usage of gawk

[Roger Pau Monné]

On Fri, Nov 24, 2023 at 12:10:57PM +0000, Alejandro Vallejo wrote:
> On 24/11/2023 11:43, Ross Lagerwall wrote:
> > On Thu, Nov 23, 2023 at 4:08 PM Roger Pau Monne <roger.pau@citrix.com>
> wrote:
> >>
> >> And instead use plain awk.
> >>
> >> There's no need to use the --non-decimal-data option for gawk, since the
> >> numbers that we want to print are already prefixed with '0x', and so
> plain awk
> >> will do the conversion from hexadecimal to decimal just fine.
> >
> > I don't think that's true (at least with gnu awk 5.1.1):
> >
> > $ echo '<e98b7>   DW_AT_byte_size   : 0x14' | awk '{printf("%d\n", $4)}'
> > 0
> >
> 
> I think it's a FreeBSD quirk. From their man page:

Seems like BusyBox inherited that behavior:

BusyBox v1.36.1 (2023-07-27 17:12:24 UTC) multi-call binary.
$ echo '<e98b7>   DW_AT_byte_size   : 0x14' | awk '{printf("%d\n", $4)}'
20

I've assumed that Busybox was the lowest common denominator, but not
in this case.

> >        Historically,  awk did not accept "0x" as a hex string. However,
> since
> >        One True Awk used strtod to convert strings to floats, and since
> "0x12"
> >        is a valid hexadecimal representation of a floating  point number,
> On
> >        FreeBSD, awk has accepted this notation as an extension since One
> True
> >        Awk was imported in FreeBSD 5.0. Upstream One True  Awk has
> restored
> >        the  historical behavior for better compatibility between the
> different
> >        awk implementations.  Both gawk and mawk already  behave similarly.
> >        Starting with FreeBSD 14.0 awk will no longer accept this
> extension.
> 
> I'm guessing the latest FreeBSD doesn't have that behaviour either?

Hm, I guess my FreeBSD world is not new enough, as the awk (version
20210724) I have does parse hex numbers:

# echo '<e98b7>   DW_AT_byte_size   : 0x14' | awk '{printf("%d\n", $4)}'
20

Will see what I can do to address this without requiring gawk if
possible.

I'm not sure if there are other issues that would prevent
livepatch-build-tools from working on FreeBSD however.

Thanks, Roger.

[PATCH 2/2] livepatch-build-tools: do not use readlink -m option

[Roger Pau Monne]

Busybox readlink implementation only supports the -f option to follow symlinks,
so adjust the logic in order to keep the same behaviour without using the -m
option.

Singed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 livepatch-build | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/livepatch-build b/livepatch-build
index 26c88a2ed2c3..2f35e98d30a3 100755
--- a/livepatch-build
+++ b/livepatch-build
@@ -336,8 +336,8 @@ while [[ $# -gt 0 ]]; do
             ;;
         --xen-syms)
             shift
-            XENSYMS="$(readlink -m -- "$1")"
-            [ -f "$XENSYMS" ] || die "xen-syms file does not exist"
+            [ -f "$1" ] || die "xen-syms file does not exist"
+            XENSYMS="$(readlink -f -- "$1")"
             shift
             ;;
         --depends)
@@ -366,22 +366,20 @@ while [[ $# -gt 0 ]]; do
 done
 
 [ -z "$srcarg" ] && die "Xen directory not given"
+[ -d "$srcarg" ] || die "Xen directory does not exist"
 [ -z "$patcharg" ] && die "Patchfile not given"
+[ -f "$patcharg" ] || die "Patchfile does not exist"
 [ -z "$configarg" ] && die ".config not given"
+[ -f "$configarg" ] || die ".config does not exist"
 [ -z "$outputarg" ] && die "Output directory not given"
 [ -z "$DEPENDS" ] && die "Build-id dependency not given"
 [ -z "$XEN_DEPENDS" ] && die "Xen Build-id dependency not given"
 
-SRCDIR="$(readlink -m -- "$srcarg")"
+SRCDIR="$(readlink -f -- "$srcarg")"
 # We need an absolute path because we move around, but we need to
 # retain the name of the symlink (= realpath -s)
 PATCHFILE="$(readlink -f "$(dirname "$patcharg")")/$(basename "$patcharg")"
-CONFIGFILE="$(readlink -m -- "$configarg")"
-OUTPUT="$(readlink -m -- "$outputarg")"
-
-[ -d "${SRCDIR}" ] || die "Xen directory does not exist"
-[ -f "${PATCHFILE}" ] || die "Patchfile does not exist"
-[ -f "${CONFIGFILE}" ] || die ".config does not exist"
+CONFIGFILE="$(readlink -f -- "$configarg")"
 
 PATCHNAME=$(make_patch_name "${PATCHFILE}")
 
@@ -390,17 +388,20 @@ echo
 echo "Xen directory: ${SRCDIR}"
 echo "Patch file: ${PATCHFILE}"
 echo ".config file: ${CONFIGFILE}"
-echo "Output directory: ${OUTPUT}"
+echo "Output directory: $outputarg"
 echo "================================================"
 echo
 
 if [ "${SKIP}" != "build" ]; then
-    [ -e "${OUTPUT}" ] && die "Output directory exists"
+    # Make sure output directory doesn't exist, and create it.
+    [ -e "$outputarg" ] && die "Output directory exists"
+    mkdir -p "$outputarg" || die
+    OUTPUT="$(readlink -f -- "$outputarg")"
+
     grep -q 'CONFIG_LIVEPATCH=y' "${CONFIGFILE}" || die "CONFIG_LIVEPATCH must be enabled"
     cd "$SRCDIR" || die
     patch -s -N -p1 -f --fuzz=0 --dry-run < "$PATCHFILE" || die "Source patch file failed to apply"
 
-    mkdir -p "${OUTPUT}" || die
     cp -f "${CONFIGFILE}" "${OUTPUT}/.config"
     cp -f "${OUTPUT}/.config" "xen/.config"
 
@@ -453,7 +454,9 @@ if [ "${SKIP}" != "build" ]; then
 fi
 
 if [ "${SKIP}" != "diff" ]; then
-    [ -d "${OUTPUT}" ] || die "Output directory does not exist"
+    cd "${SCRIPTDIR}" || die
+    [ -d "$outputarg" ] || die "Output directory does not exist"
+    OUTPUT="$(readlink -f -- "$outputarg")"
 
     cd "${OUTPUT}" || die
     create_patch
-- 
2.43.0

Re: [PATCH 2/2] livepatch-build-tools: do not use readlink -m option

[Ross Lagerwall]

On Thu, Nov 23, 2023 at 4:08 PM Roger Pau Monne <roger.pau@citrix.com> wrote:
>
> Busybox readlink implementation only supports the -f option to follow symlinks,
> so adjust the logic in order to keep the same behaviour without using the -m
> option.
>
> Singed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>

Thanks