RE: [bitbake-devel] [PATCH] Added double quotes aound variables in bitbake/source to prevent gobbling or splitting

[Peter Kjellerstedt <peter.kjellerstedt@axis.com>]

[ Since you seem new to the process of reviewing via mail, please 
  note that I have added comments throughout the entire patch below. ]

> -----Original Message-----
> From: bitbake-devel@lists.openembedded.org <bitbake-
> devel@lists.openembedded.org> On Behalf Of Abongwa Amahnui Bonalais
> Sent: den 5 april 2022 20:29
> To: bitbake-devel@lists.openembedded.org
> Cc: Abongwa Bonalais Amahnui <abongwabonalais@gmail.com>
> Subject: [bitbake-devel] [PATCH] Added double quotes aound variables in bitbake/source to prevent gobbling or splitting

Always prefix the subject line with the affected recipe name or similar. 
Also keep the subject short, typically shorter than 72 characters.
A better subject in this case would be:

  toaster: Add quoting of variables to avoid splitting

Then there should be a description that gives a motivation for 
why this is needed and any other relevant information.

> 
> Signed-off-by: Abongwa Bonalais Amahnui <abongwabonalais@gmail.com>
> ---
>  bitbake/bin/toaster | 65 +++++++++++++++++++++++----------------------
>  1 file changed, 33 insertions(+), 32 deletions(-)
> 
> diff --git a/bitbake/bin/toaster b/bitbake/bin/toaster
> index 558a819570..d98c4c69f0 100755
> --- a/bitbake/bin/toaster
> +++ b/bitbake/bin/toaster
> @@ -1,4 +1,5 @@
> -#!/bin/echo ERROR: This script needs to be sourced. Please run as .
> +#!/bin/sh
> +echo ERROR: This script needs to be sourced. Please run as .

Leave as it was. It is explicitly _not_ using #!/bin/sh as shebang line 
to catch misuse.

> 
>  # toaster - shell script to start Toaster
> 
> @@ -20,8 +21,8 @@ Usage 2: source toaster manage [createsuperuser|lsupdates|migrate|makemigrations
>  custom_extention()
>  {
> 
> custom_extension=$BBBASEDIR/lib/toaster/orm/fixtures/custom_toaster_append.sh
> -    if [ -f $custom_extension ] ; then
> -        $custom_extension $*
> +    if [ -f "$custom_extension" ] ; then
> +        $custom_extension "$*"
>      fi
>  }
> 
> @@ -51,19 +52,19 @@ databaseCheck()
>  webserverKillAll()
>  {
>      local pidfile
> -    if [ -f ${BUILDDIR}/.toastermain.pid ] ; then
> +    if [ -f "${BUILDDIR}"/.toastermain.pid ] ; then

While it is technically correct, please put the quotes around the 
entire string to make it more readable, i.e.:

    if [ -f "${BUILDDIR}/.toastermain.pid" ] ; then

This applies to the rest of the changes in this file.

>          custom_extention web_stop_postpend
>      else
>          custom_extention noweb_stop_postpend
>      fi
> -    for pidfile in ${BUILDDIR}/.toastermain.pid ${BUILDDIR}/.runbuilds.pid; do
> -        if [ -f ${pidfile} ]; then
> -            pid=`cat ${pidfile}`
> -            while kill -0 $pid 2>/dev/null; do
> -                kill -SIGTERM $pid 2>/dev/null
> +    for pidfile in "${BUILDDIR}"/.toastermain.pid "${BUILDDIR}"/.runbuilds.pid; do
> +        if [ -f "${pidfile}" ]; then
> +            pid=`cat "${pidfile}"`
> +            while kill -0 "$pid" 2>/dev/null; do
> +                kill -SIGTERM "$pid" 2>/dev/null
>                  sleep 1
>              done
> -            rm  ${pidfile}
> +            rm  "${pidfile}"
>          fi
>      done
>  }
> @@ -84,8 +85,8 @@ webserverStartAll()
>      echo "Starting webserver..."
> 
>      $MANAGE runserver --noreload "$ADDR_PORT" \
> -           </dev/null >>${BUILDDIR}/toaster_web.log 2>&1 \
> -           & echo $! >${BUILDDIR}/.toastermain.pid
> +           </dev/null >>"${BUILDDIR}"/toaster_web.log 2>&1 \
> +           & echo "$!" >"${BUILDDIR}"/.toastermain.pid
> 
>      sleep 1
> 
> @@ -95,7 +96,7 @@ webserverStartAll()
>      else
>          echo "Toaster development webserver started at http://$ADDR_PORT"
>          echo -e "\nYou can now run 'bitbake <target>' on the command line and monitor your build in Toaster.\nYou can also use a Toaster project to configure and run a build.\n"
> -        custom_extention web_start_postpend $ADDR_PORT
> +        custom_extention web_start_postpend "$ADDR_PORT"
>      fi
> 
>      return $retval
> @@ -131,8 +132,8 @@ verify_prereq() {
>      exp=$exp'vmax=["%02d" % int(n) for n in "\4".split(".")];'
>      exp=$exp'sys.exit(not (version \1 vmin and version \3 vmax))'
>      exp=$exp'/p'
> -    if ! sed -n "$exp" $reqfile | python3 - ; then
> -        req=`grep ^Django $reqfile`
> +    if ! sed -n "$exp" "$reqfile" | python3 - ; then
> +        req=`grep ^Django "$reqfile"`
>          echo "This program needs $req"
>          echo "Please install with pip3 install -r $reqfile"
>          return 2
> @@ -150,10 +151,10 @@ else
>      TOASTER=$0
>  fi
> 
> -export BBBASEDIR=`dirname $TOASTER`/..
> +export BBBASEDIR=`dirname "$TOASTER"`/..
>  MANAGE="python3 $BBBASEDIR/lib/toaster/manage.py"
>  if [ -z "$OE_ROOT" ]; then
> -    OE_ROOT=`dirname $TOASTER`/../..
> +    OE_ROOT=`dirname "$TOASTER"`/../..
>  fi
> 
>  # this is the configuraton file we are using for toaster
> @@ -164,7 +165,7 @@ fi
>  # in the local layers that currently make using an arbitrary
>  # toasterconf.json difficult.
> 
> -. $OE_ROOT/.templateconf
> +. "$OE_ROOT"/.templateconf
>  if [ -n "$TEMPLATECONF" ]; then
>      if [ ! -d "$TEMPLATECONF" ]; then
>          # Allow TEMPLATECONF=meta-xyz/conf as a shortcut
> @@ -199,10 +200,10 @@ for param in $*; do
>      webport=*)
>              ADDR_PORT="${param#*=}"
>              # Split the addr:port string
> -            ADDR=`echo $ADDR_PORT | cut -f 1 -d ':'`
> -            PORT=`echo $ADDR_PORT | cut -f 2 -d ':'`
> +            ADDR=`echo "$ADDR_PORT" | cut -f 1 -d ':'`
> +            PORT=`echo "$ADDR_PORT" | cut -f 2 -d ':'`
>              # If only a port has been speified then set address to localhost.
> -            if [ $ADDR = $PORT ] ; then
> +            if [ "$ADDR" = "$PORT" ] ; then
>                  ADDR_PORT="localhost:$PORT"
>              fi
>      ;;
> @@ -229,7 +230,7 @@ for param in $*; do
>      esac
>  done
> 
> -if [ `basename \"$0\"` = `basename \"${TOASTER}\"` ]; then
> +if [ `basename \""$0"\"` = `basename \""${TOASTER}"\"` ]; then

That is wrong (as the original code was wrong). Change it to:

if [ `basename "$0"` = `basename "${TOASTER}"` ]; then

>      echo "Error: This script needs to be sourced. Please run as . $TOASTER"
>      return 1
>  fi
> @@ -265,7 +266,7 @@ fi
>  echo "The system will $CMD."
> 
>  # Execute the commands
> -custom_extention toaster_prepend $CMD $ADDR_PORT
> +custom_extention toaster_prepend "$CMD" "$ADDR_PORT"
> 
>  case $CMD in
>      start )
> @@ -279,7 +280,7 @@ case $CMD in
>          # Create configuration file
>          conf=${BUILDDIR}/conf/local.conf
>          line='INHERIT+="toaster buildhistory"'
> -        grep -q "$line" $conf || echo $line >> $conf
> +        grep -q "$line" "$conf" || echo "$line" >> "$conf"
> 
>          if [ $WEBSERVER -eq 0 ] ; then
>              # Do not update the database for "noweb" unless
> @@ -290,17 +291,17 @@ case $CMD in
>                      return 4
>                  fi
>              fi
> -            custom_extention noweb_start_postpend $ADDR_PORT
> +            custom_extention noweb_start_postpend "$ADDR_PORT"
>          fi
> -        if [ $WEBSERVER -gt 0 ] && ! webserverStartAll; then
> +        if [ "$WEBSERVER" -gt 0 ] && ! webserverStartAll; then
>              echo "Failed ${CMD}."
>              return 4
>          fi
>          export BITBAKE_UI='toasterui'
>          if [ $TOASTER_BUILDSERVER -eq 1 ] ; then
>              $MANAGE runbuilds \
> -               </dev/null >>${BUILDDIR}/toaster_runbuilds.log 2>&1 \
> -               & echo $! >${BUILDDIR}/.runbuilds.pid
> +               </dev/null >>"${BUILDDIR}"/toaster_runbuilds.log 2>&1 \
> +               & echo $! >"${BUILDDIR}"/.runbuilds.pid
>          else
>              echo "Toaster build server not started."
>          fi
> @@ -308,7 +309,7 @@ case $CMD in
>          # set fail safe stop system on terminal exit
>          trap stop_system SIGHUP
>          echo "Successful ${CMD}."
> -        custom_extention toaster_postpend $CMD $ADDR_PORT
> +        custom_extention toaster_postpend "$CMD" "$ADDR_PORT"
>          return 0
>      ;;
>      stop )
> @@ -316,9 +317,9 @@ case $CMD in
>          echo "Successful ${CMD}."
>      ;;
>      manage )
> -        cd $BBBASEDIR/lib/toaster
> -        $MANAGE $manage_cmd
> +        cd "$BBBASEDIR"/lib/toaster
> +        "$MANAGE $manage_cmd"
>      ;;
>  esac
> -custom_extention toaster_postpend $CMD $ADDR_PORT
> +custom_extention toaster_postpend "$CMD" "$ADDR_PORT"
> 
> --
> 2.25.1

//Peter