Hi,
The attached diff goal is to make the 'rumprun' shell script a bit
more robust, mainly for the QEMU command generation (so QEMU and KVM
platforms).
Actually, multiples variables corresponding to options that will be
later passed to qemu-system-* commands are generated by rumprun and
the final command is generated from that. One of the problems that I
encountered is sometimes, depending on the passed filenames or
options, the command generation can break.
An example is for the '-drive' options that is currently generated as follows:
opt_drivespec="${opt_drivespec} -drive if=virtio,file=${image}"
Here, if $image contains some special characters (like spaces) the
qemu command generation will break due to later shell interpretation
(truncated example for clarity):
qemucmd="[...] ${qemu} [...] ${opt_drivespec} [...]"
[...]
${qemucmd}
The attached diff handles the command generation through xargs -0: the
options are simply printed and piped so no shell interpretation can
occur.
However to allow the use of xargs, and this is the reason of this post
on the mailing list, the rumprun cli changes from:
rumprun [script args] PLATFORM [guest args] APP [ -- ] [app args]
to:
rumprun [script args] PLATFORM APP [guest args] [ -- ] [app args]
Furthermore, the [app args] part is now handled as a JSON array in the
configuration, avoiding superfluous quoting, see the issue 40 on
github that also discussed a bit and shaped this diff:
https://github.com/rumpkernel/rumprun/issues/40
Finally, I would like to discuss the possibility to auto-detect the
platform from the application. As you could see from the discussion on
issue 40, adding special information to the ELF binary could allow
rumprun to defaults to using QEMU for the hardware platform or Xen for
the Xen platform by default (see the discussed implementation in the
issue 40 that is not part of this diff here:
https://github.com/rumpkernel/rumprun/commit/e8e6bb7ccf8cd12cff26d7b92bb3cc8391050272,
note that instead of defsym ELF notes should be used, but this
illustrate the proposition).
A downside being that if KVM wanted to be used (or another platform
that could be added in the future like VirtualBox), the platform
should be specified explicitly with an option to rumprun (which will
roughly be the same as today).
The final rumprun cli could look like this:
rumprun [script args] [ -- ] APP [guest args] [ -- ] [app args]
I'll be happy to hear any suggestions/critics/remarks/...
Regards.
diff --git a/app-tools/rumprun b/app-tools/rumprun
index 6fcd5ba..f41b7e7 100755
--- a/app-tools/rumprun
+++ b/app-tools/rumprun
@@ -29,10 +29,12 @@
# rumprun: "driver" script for running rumprun application stacks
#
+set -u
+
# default values
MEM_DEFAULT=64
-if [ "${RUMPRUN_WARNING_STFU}" != 'please' ]; then
+if [ "${RUMPRUN_WARNING_STFU:-}" != 'please' ]; then
exec 3>&1 1>&2
echo
echo !!!
@@ -63,7 +65,7 @@ usage ()
{
cat <<EOM
-usage: rumprun [script args] PLATFORM [guest args] APP [ -- ] [app args]
+usage: rumprun [script args] PLATFORM APP [guest args] [ -- ] [app args]
PLATFORM is the rumprun runtime platform to use
iso : bake app and config into a bootable iso image
@@ -127,20 +129,87 @@ nuketmpdir ()
setuptmpdir ()
{
- if [ -z "${TMPDIR}" ]; then
+ if [ -z "${TMPDIR:-}" ]; then
TMPDIR=$(mktemp -d /tmp/rumprun.XXXXXXXX)
else
- [ ! -d "${TMPDIR}" ] || die ${TMPDIR} already exists
- mkdir -p "${TMPDIR}" || die could not create ${TMPDIR}
- chmod 700 "${TMPDIR}" \
- || { rm -rf "${TMPDIR}" ; die could not chmod ${TMPDIR}; }
+ [ ! -d "${TMPDIR}" ] || die "${TMPDIR} already exists"
+ mkdir -p "${TMPDIR}" || die "could not create ${TMPDIR}"
+ chmod 700 "${TMPDIR}" ||
+ { rm -rf "${TMPDIR}" ; die "could not chmod ${TMPDIR}"; }
fi
trap nuketmpdir 0 INT TERM
}
+command_add_parameters ()
+{
+ for parameter in "$@"; do
+ printf "%s\0" "${parameter}"
+ done
+}
+
+command_add_user_parameters ()
+{
+ # if the user supplied custom parameters (-g option for qemu), like
+ # -g "-curses -someoption '...'"
+ # it will interpret it as
+ # '-curses', '-someoption', '...'
+ eval "set -- $@"
+ command_add_parameters "$@"
+}
+
+# should only output on stdout the parameters for generating the final command
+command_worker ()
+{
+ local func
+
+ [ -n "${1:-}" ] || die 'command_worker(): setup function not set'
+ func=$1; shift
+
+ "${func}" "$@"
+}
+
+command_exec ()
+{
+ local program func
+
+ # program to execute
+ [ -n "${1:-}" ] || die 'command_exec(): program not set, nothing to
execute'
+ program=$1; shift
+
+ # function to call that will write to pipe
+ [ -n "${1:-}" ] || die 'command_exec(): setup function not set'
+ func=$1; shift
+
+ if [ -n "${DUMPCMD:-}" ]; then
+ command_worker "${func}" "$@" | xargs -0 ${DUMPCMD} "${program}"
+ else
+ # everything should properly execute before even trying the
command
+ command_worker "${func}" "$@" > /dev/null || die "an error
occured"
+ command_worker "${func}" "$@" | {
+ if [ -z "${opt_interactive:-}" ]; then
+ xargs -0 "${program}" 1>/dev/null 2>&1
+ else
+ xargs -0 "${program}"
+ fi
+ }
+ fi
+}
+
+check_file ()
+{
+ local file
+ file=$(readlink -f "$1")
+ [ -z "${file}" -o ! -f "${file}" ] && die "$1: file does not exist"
+}
+
+quote_json ()
+{
+ printf "%s" "$1" | sed -e 's/\(["]\)/\\\1/g'
+}
+
check_app ()
{
- [ -f "$1" ] || die "file not found: $1"
+ check_file "$1"
if ! grep -q __RuMpRuN_baked__ "$1"; then
die "$1: not a rumprun unikernel (did you forget rumpbake?)"
@@ -162,7 +231,9 @@ createif_qemu ()
ifbasename="${scratch%%,*}"
qemuargs="${scratch#*,}"
- opt_netif="${opt_netif} -net nic,model=virtio ${qemuargs}"
+ command_add_parameters "-net" "nic,model=virtio"
+ command_add_user_parameters "${qemuargs}"
+
eval ${iftag}2ifname=${ifbasename}${nindex}
eval ${iftag}2cloner=false
nindex=$(expr $nindex + 1)
@@ -212,7 +283,7 @@ json_indent ()
{
for x in $(seq ${json_depth}); do
- printf '\t' >> ${TMPDIR}/json.cfg
+ printf '\t' >> "${TMPDIR}/json.cfg"
done
}
@@ -220,7 +291,27 @@ json_append_ln ()
{
json_indent
- echo "$*, " >> ${TMPDIR}/json.cfg
+ echo "$*, " >> "${TMPDIR}/json.cfg"
+}
+
+json_append_array ()
+{
+ local array_name
+
+ array_name=$1;
+ shift
+ {
+ json_indent;
+ echo "\"${array_name}\": [, "
+ json_depth=$((${json_depth}+1))
+ for elem in "$@"; do
+ json_indent;
+ echo "\"${elem}\", "
+ done
+ json_depth=$((${json_depth}-1))
+ json_indent;
+ echo "], "
+ } >> "${TMPDIR}/json.cfg"
}
json_open_block ()
@@ -230,6 +321,12 @@ json_open_block ()
json_depth=$((${json_depth}+1))
}
+json_begin ()
+{
+ > "${TMPDIR}/json.cfg"
+ json_open_block
+}
+
json_finalize_block ()
{
@@ -238,6 +335,13 @@ json_finalize_block ()
json_append_ln }
}
+json_end ()
+{
+ json_finalize_block
+ [ ${json_depth} -eq 0 ] ||
+ die "json creation failed, internal error (final depth:
${json_depth})"
+}
+
# Detect the filesystem type of the image at $1.
detect_fstype ()
{
@@ -271,23 +375,23 @@ json_store_netspec ()
json_open_block net
# XXX: should not assume vioif or -net user
- json_append_ln '"if": "'$(eval echo \${${iftag}2ifname})'"'
+ json_append_ln "\"if\": \"$(eval echo
\${${iftag}2ifname})\""
if $(eval \${${iftag}2cloner}); then
json_append_ln '"cloner": "'true'"'
fi
- json_append_ln '"type": "'${iftype}'"'
- json_append_ln '"method": "'${ifmethod}'"'
+ json_append_ln "\"type\": \"${iftype}\""
+ json_append_ln "\"method\": \"${ifmethod}\""
[ -n "${ifaddr}" ] || { json_finalize_block ; return ; }
- json_append_ln '"addr": "'${ifaddr}'"'
- json_append_ln '"mask": "'${ifmask}'"'
+ json_append_ln "\"addr\": \"${ifaddr}\""
+ json_append_ln "\"mask\": \"${ifmask}\""
- [ -n "${ifgw}" ] || { json_finalize_block ; return ; }
+ [ -n "${ifgw:-}" ] || { json_finalize_block ; return ; }
- json_append_ln '"gw": "'${ifgw}'"'
+ json_append_ln "\"gw\": \"${ifgw}\""
json_finalize_block
}
@@ -315,7 +419,7 @@ parse_netspec ()
ifmask=${3#*/}
[ -n "${ifaddr}" ] || usage
[ -n "${ifmask}" ] || usage
- ifgw=$4
+ ifgw=${4:-}
;;
*)
return 1
@@ -343,13 +447,13 @@ json_store_xen_blkspec ()
json_open_block blk
json_append_ln '"source": "'etfs'"'
- json_append_ln '"path": "'${vdev}'"'
+ json_append_ln "\"path\": \"${vdev}\""
# if the image does not need to be mounted, we're done
[ -n "${mountpoint}" ] || { json_finalize_block ; return ; }
- json_append_ln '"fstype": "'${fstype}'"'
- json_append_ln '"mountpoint": "'${mountpoint}'"'
+ json_append_ln "\"fstype\": \"${fstype}\""
+ json_append_ln "\"mountpoint\": \"${mountpoint}\""
json_finalize_block
}
@@ -358,7 +462,7 @@ json_store_qemu_blkspec ()
{
# XXX: should not generate the interface here
- opt_drivespec="${opt_drivespec} -drive if=virtio,file=${image}"
+ command_add_parameters "-drive" "if=virtio,file=${image}"
# if the image does not need to be mounted, we're done
[ -n "${mountpoint}" ] || return
@@ -367,9 +471,9 @@ json_store_qemu_blkspec ()
# XXX: should not assume ld (virtio)
json_append_ln '"source": "'dev'"'
- json_append_ln '"path": "'/dev/ld${bindex}a'"'
- json_append_ln '"fstype": "'${fstype}'"'
- json_append_ln '"mountpoint": "'${mountpoint}'"'
+ json_append_ln "\"path\": \"/dev/ld${bindex}a\""
+ json_append_ln "\"fstype\": \"${fstype}\""
+ json_append_ln "\"mountpoint\": \"${mountpoint}\""
json_finalize_block
}
@@ -381,19 +485,19 @@ json_store_iso_blkspec ()
# XXX: might have accidental clashes due to images from
# many source subdirectories
- [ ! -f "${ISODIR}/images/$(basename ${image})" ] \
- || die image \"${image}\" already exists in destdir
- cp ${image} "${ISODIR}/images/"
+ [ ! -f "${ISODIR}/images/$(basename "${image}")" ] ||
+ die "image \"${image}\" already exists in destdir"
+ cp "${image}" "${ISODIR}/images/"
# well, this is a bit questionable, but ...
[ -n "${mountpoint}" ] || return
json_open_block blk
- json_append_ln '"source": "'vnd'"'
- json_append_ln '"path": "'images/$(basename ${image})'"'
- json_append_ln '"fstype": "'${fstype}'"'
- json_append_ln '"mountpoint": "'${mountpoint}'"'
+ json_append_ln "\"source\": \"vnd\""
+ json_append_ln "\"path\": \"images/$(basename "${image}")\""
+ json_append_ln "\"fstype\": \"$fstype\""
+ json_append_ln "\"mountpoint\": \"$mountpoint\""
json_finalize_block
}
@@ -416,17 +520,17 @@ parse_blkspec ()
spec=$1
image_path=${spec%,*}
- [ -n "$image_path" ] || usage
- image=$(readlink -f "$image_path")
- [ -f "$image" ] || die "File $image_path does not exist"
- mountpoint=$(echo "$spec" | sed -n 's/.*,\(.*\)/\1/p')
+ [ -n "${image_path}" ] || usage
+ image=${image_path}
+ check_file "${image}"
+ mountpoint=$(echo "${spec}" | sed -n 's/.*,\(.*\)/\1/p')
if [ -n "$mountpoint" ]; then
- fstype=$(detect_fstype "$image")
- [ $? -ne 0 ] && die "File $image: unknown fstype"
+ fstype=$(detect_fstype "${image}")
+ [ $? -ne 0 ] && die "File ${image}: unknown fstype"
fi
- ${store_blkspec}
+ "${store_blkspec}"
bindex=$(expr $bindex + 1)
}
@@ -445,6 +549,11 @@ run_xen ()
fi
done
+ app=$1
+ shift
+ check_app "$app"
+ opt_name=rumprun-$(basename "$app")
+
store_blkspec=json_store_xen_blkspec
store_netspec=json_store_netspec
@@ -458,11 +567,10 @@ run_xen ()
opt_destroy='on_poweroff="preserve"'
opt_destroy_crash='on_crash="preserve"'
opt_mem=${MEM_DEFAULT}
- opt_name=
opt_debug=
sudo=''
- json_open_block
+ json_begin
while getopts "${guestopts}" opt; do
case "$opt" in
@@ -525,25 +633,18 @@ run_xen ()
# Remaining arguments belong to the application.
shift $((OPTIND-1))
- [ "$1" = "--" ] && shift
- [ $# -lt 1 ] && usage
# kernfs is always present on Xen
json_store_kernfs
- json_append_ln "\"cmdline\": \""$@"\""
- [ -n "${opt_name}" ] && json_append_ln "\"hostname\": \""${opt_name}"\""
- json_finalize_block
+ json_append_array "cmdline" "$(quote_json "${app}")" "$@"
+ [ -n "${opt_name}" ] && json_append_ln "\"hostname\": \"${opt_name}\""
+ json_end
- name=${opt_name:-rumprun-$(basename "$1")}
- app=$(readlink -f "$1")
- [ -f "$app" ] || die "rumprun unikernel $1 does not exist"
- shift
- check_app ${app}
# Generate xl configuration file.
cat <<EOM >"${TMPDIR}/xr.conf"
kernel="${app}"
-name="${name}"
+name="${opt_name}"
vcpus=1
memory=${opt_mem}
${opt_destroy}
@@ -556,14 +657,14 @@ EOM
fi
# Create the domain and leave it paused so that we can get its domid.
- if ! ${DUMPCMD} ${sudo} xl create -p ${conf} >/dev/null; then
+ if ! ${DUMPCMD} ${sudo} xl create -p "${conf}" >/dev/null; then
die xl create failed
fi
- domid=$(${DUMPCMD} ${sudo} xl domid ${name})
+ domid=$(${DUMPCMD} ${sudo} xl domid "${opt_name}")
# Write provisioning information for domain to xenstore.
prefix=/local/domain/${domid}/rumprun
${DUMPCMD} ${sudo} xenstore-write \
- "${prefix}/cfg" "$(cat ${TMPDIR}/json.cfg)"
+ "${prefix}/cfg" "$(cat "${TMPDIR}/json.cfg")"
nuketmpdir
@@ -585,24 +686,23 @@ EOM
fi
}
-run_qemu ()
+configure_qemu ()
{
+ local opt_mem
store_blkspec=json_store_qemu_blkspec
store_netspec=json_store_netspec
opt_interactive=
- opt_drivespec=
- opt_debug=
- opt_pause=
- opt_netif=
- opt_guestargs=
opt_mem=${MEM_DEFAULT}
- json_open_block
+ json_begin
variant=$1
- shift
+ app_file=$2
+ pid_file=$3
+ shift 3
+
while getopts "${guestopts}" opt; do
case "$opt" in
b)
@@ -612,14 +712,14 @@ run_qemu ()
die -d not supported on qemu/kvm
;;
D)
- opt_debug="-gdb tcp::${OPTARG}"
+ command_add_parameters "-gdb" "tcp::$OPTARG"
;;
e)
+ # an array could be better
json_append_ln "\"env\": \"${OPTARG}\""
;;
g)
- # toimii ku gunan vessa
- opt_guestargs="${opt_guestargs} ${OPTARG}"
+ command_add_user_parameters "${OPTARG}"
;;
i)
opt_interactive=1
@@ -642,7 +742,7 @@ run_qemu ()
opt_name=${OPTARG}
;;
p)
- opt_pause="-S"
+ command_add_parameters "-S"
;;
W)
parse_newnetspec "${OPTARG}" || usage
@@ -655,41 +755,52 @@ run_qemu ()
shift $((${OPTIND}-1))
if [ "${variant}" = "kvm" ]; then
- opt_kvm="-enable-kvm -cpu host"
+ command_add_parameters "-enable-kvm" "-cpu" "host"
else
- opt_kvm=-no-kvm
+ command_add_parameters "-no-kvm"
fi
- json_append_ln "\"cmdline\": \""$@"\""
- [ -n "${opt_name}" ] && json_append_ln "\"hostname\": \""${opt_name}"\""
- json_finalize_block
-
- # internal check
- [ ${json_depth} -eq 0 ] || die internal error, final depth ${json_depth}
+ json_append_array "cmdline" "$(quote_json "${app_file}")" "$@"
+ [ -n "${opt_name:-}" ] && json_append_ln "\"hostname\": \"${opt_name}\""
+ json_end
json_coma="$(sed 's/,/,,/g' < "${TMPDIR}/json.cfg")"
- [ -n "${opt_netif}" ] || opt_netif="-net none"
+ command_add_parameters "-m" "${opt_mem}"
+ command_add_parameters "-kernel" "${app_file}"
+ command_add_parameters "-append" "${json_coma}"
+
+ if [ -z "$opt_interactive" ]; then
+ command_add_parameters "-pidfile" "${pid_file}" "-daemonize"
+ command_add_parameters "-display" "none"
+ fi
+}
+
+run_qemu ()
+{
+ local variant app_file pid_file
+
+ variant=$1
+ app_file=$2
+ shift 2
+ pid_file="${TMPDIR}/qemu.pid"
- check_app $1
+ check_app "${app_file}"
# XXX: using host objdump here is wrong, but xen uses it too,
# and both offenses should be easy to fix at the same time
- if objdump -f ${1} | grep -q elf64; then
+ if objdump -f "${app_file}" | grep -q "elf64"; then
qemu=qemu-system-x86_64
else
qemu=qemu-system-i386
fi
- qemucmd="${DUMPCMD} ${qemu} ${opt_netif} ${opt_kvm} \
- ${opt_drivespec} ${opt_debug} ${opt_pause} -m ${opt_mem} \
- ${opt_guestargs} -kernel $1"
- if [ -n "$opt_interactive" ]; then
- ${qemucmd} -append "${json_coma}"
- else
- qemucmd="${qemucmd} -display none"
- ${qemucmd} -append "${json_coma}" 1>/dev/null 2>&1 &
- echo qemu:$!
+ command_exec "${qemu}" \
+ configure_qemu "${variant}" "${app_file}"
"${pid_file}" "$@"
+
+ if [ -f "${pid_file}" ]; then
+ echo "qemu:$(cat "${pid_file}")"
+ return
fi
}
@@ -699,14 +810,14 @@ bake_iso ()
type xorriso >/dev/null || die bake_iso needs xorriso
type grub-mkrescue >/dev/null || die bake_iso needs grub-mkrescue
+ bootimage=$1
+ check_app "${bootimage}"
+ shift
+
store_blkspec=json_store_iso_blkspec
store_netspec=json_store_netspec
- opt_interactive=
- opt_drivespec=
- opt_debug=
- opt_pause=
- opt_netif=
+ opt_name=rumprun-"$(echo "${bootimage}" | tr / _)"
opt_mem=${MEM_DEFAULT}
blk_specified=false
@@ -714,7 +825,7 @@ bake_iso ()
ISODIR="${TMPDIR}/iso"
mkdir -p "${ISODIR}"
- json_open_block
+ json_begin
while getopts "${guestopts}" opt; do
case "$opt" in
@@ -758,26 +869,26 @@ bake_iso ()
esac
done
shift $((${OPTIND}-1))
- bootimage=$1
- check_app ${bootimage}
- : ${opt_name:=rumprun-$(echo ${bootimage} | tr / _).iso}
- [ ! -f ${opt_name} ] || die target ${opt_name} already exists
+ opt_name=${opt_name%.iso}
+ iso_name=${opt_name}.iso
+ [ ! -f "${iso_name}" ] || die "target ${iso_name} already exists"
+ json_append_ln "\"hostname\": \"${opt_name}\""
- json_append_ln "\"cmdline\": \""$@"\""
- [ -n "${opt_name}" ] && json_append_ln "\"hostname\": \""${opt_name}"\""
- json_finalize_block
- [ ${json_depth} -eq 0 ] || die internal error, final depth ${json_depth}
+ json_append_array "cmdline" "$(quote_json "${bootimage}")" "$@"
+ json_end
# create the iso directory structure
mkdir -p "${ISODIR}/boot/grub"
- printf 'set timeout=0\n' > "${ISODIR}/boot/grub/grub.cfg"
- printf 'menuentry "rumpkernel" {\n' >> "${ISODIR}/boot/grub/grub.cfg"
- printf '\tmultiboot /boot/%s ROOTFSCFG=/json.cfg\n}\n' $(basename $1) \
- >> "${ISODIR}/boot/grub/grub.cfg"
- cp ${bootimage} "${ISODIR}/boot"
+ cat << EOF > "${ISODIR}/boot/grub/grub.cfg"
+set timeout=0
+menuentry "rumpkernel" {
+ multiboot /boot/$(basename "${bootimage}") ROOTFSCFG=/json.cfg
+}
+EOF
+ cp "${bootimage}" "${ISODIR}/boot"
cp "${TMPDIR}/json.cfg" "${ISODIR}"
- grub-mkrescue "${ISODIR}" -o ${opt_name}
+ grub-mkrescue "${ISODIR}" -o "${iso_name}"
}
if [ $# -lt 2 ]; then
@@ -812,7 +923,7 @@ shift
setuptmpdir
-case ${platform} in
+case "${platform}" in
xen)
run_xen "$@"
;;
diff --git a/include/bmk-core/jsmn.h b/include/bmk-core/jsmn.h
index 247bfa2..6b11ada 100644
--- a/include/bmk-core/jsmn.h
+++ b/include/bmk-core/jsmn.h
@@ -23,6 +23,8 @@
#ifndef _BMK_CORE_JSMN_H_
#define _BMK_CORE_JSMN_H_
+#define JSMN_PARENT_LINKS
+
#ifdef __cplusplus
extern "C" {
#endif
@@ -82,7 +84,7 @@ typedef struct {
void jsmn_init(jsmn_parser *parser);
/**
- * Run JSON parser. It parses a JSON data string into and array of tokens,
each describing
+ * Run JSON parser. It parses a JSON data string into an array of tokens, each
describing
* a single JSON object.
*/
jsmnerr_t jsmn_parse(jsmn_parser *parser, const char *js, unsigned long len,
diff --git a/include/rumprun-base/parseargs.h b/include/rumprun-base/parseargs.h
deleted file mode 100644
index 95a5742..0000000
--- a/include/rumprun-base/parseargs.h
+++ /dev/null
@@ -1,31 +0,0 @@
-/*-
- * Copyright (c) 2014 Citrix. All Rights Reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS
- * OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
- * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
- * DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
- * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
- * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
- * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
- * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
- * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- */
-
-#ifndef _RUMPRUN_BASE_PARSEARGS_H_
-#define _RUMPRUN_BASE_PARSEARGS_H_
-
-void rumprun_parseargs(char *, int *, char **);
-
-#endif /* _RUMPRUN_BASE_PARSEARGS_H_ */
diff --git a/lib/librumprun_base/Makefile b/lib/librumprun_base/Makefile
index 6fd3bb6..4a5df3c 100644
--- a/lib/librumprun_base/Makefile
+++ b/lib/librumprun_base/Makefile
@@ -2,7 +2,7 @@ LIB= rumprun_base
LIBISPRIVATE= # defined
SRCS= rumprun.c
-SRCS+= parseargs.c rumpconfig.c
+SRCS+= rumpconfig.c
SRCS+= malloc.c netbsd_initfini.c signals.c
SRCS+= syscall_mman.c syscall_misc.c
SRCS+= __errno.c _lwp.c libc_stubs.c
diff --git a/lib/librumprun_base/parseargs.c b/lib/librumprun_base/parseargs.c
deleted file mode 100644
index 4965781..0000000
--- a/lib/librumprun_base/parseargs.c
+++ /dev/null
@@ -1,74 +0,0 @@
-/*-
- * Copyright (c) 2014 Citrix. All Rights Reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS
- * OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
- * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
- * DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
- * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
- * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
- * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
- * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
- * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- */
-
-#include <rumprun-base/parseargs.h>
-
-void
-rumprun_parseargs(char *p, int *nargs, char **outarray)
-{
- char *out = 0;
- int quote = -1; /* -1 means outside arg, 0 or '"' or '\'' inside */
-
- *nargs = 0;
-
- for (;;) {
- int ac = *p++;
- int rc = ac;
- if (ac == '\\')
- rc = *p++;
- if (!rc || ac==' ' || ac=='\n' || ac=='\t') {
- /* any kind of delimiter */
- if (!rc || quote==0) {
- /* ending an argument */
- if (out)
- *out++ = 0;
- quote = -1;
- }
- if (!rc)
- /* definitely quit then */
- break;
- if (quote<0)
- /* not in an argument now */
- continue;
- }
- if (quote<0) {
- /* starting an argument */
- if (outarray)
- outarray[*nargs] = out = p-1;
- (*nargs)++;
- quote = 0;
- }
- if (quote > 0 && ac == quote) {
- quote = 0;
- continue;
- }
- if (ac == '\'' || ac == '"') {
- quote = ac;
- continue;
- }
- if (out)
- *out++ = rc;
- }
-}
diff --git a/lib/librumprun_base/rumpconfig.c b/lib/librumprun_base/rumpconfig.c
index 0a34df3..403f82f 100644
--- a/lib/librumprun_base/rumpconfig.c
+++ b/lib/librumprun_base/rumpconfig.c
@@ -51,7 +51,6 @@
#include <rump/netconfig.h>
#include <rumprun-base/config.h>
-#include <rumprun-base/parseargs.h>
#include <bmk-core/jsmn.h>
@@ -96,33 +95,67 @@ int rumprun_cmdline_argc;
char **rumprun_cmdline_argv;
static void
-makeargv(char *argvstr)
+makedefaultargv(void)
{
char **argv;
- int nargs;
assert(rumprun_cmdline_argc == 0 && rumprun_cmdline_argv == NULL);
- rumprun_parseargs(argvstr, &nargs, 0);
- argv = malloc(sizeof(*argv) * (nargs+2));
+ argv = malloc(sizeof(*argv) * 2);
if (argv == NULL)
errx(1, "could not allocate argv");
- rumprun_parseargs(argvstr, &nargs, argv);
- argv[nargs] = argv[nargs+1] = '\0';
+ argv[0] = "rumprun";
+ argv[1] = NULL;
+ rumprun_cmdline_argv = argv;
+ rumprun_cmdline_argc = 1;
+}
+
+static void
+makeargv(jsmntok_t *t, char *data, int argc)
+{
+ char **argv;
+
+ assert(rumprun_cmdline_argc == 0 && rumprun_cmdline_argv == NULL);
+
+ argv = malloc(sizeof(*argv) * (argc + 1));
+ if (argv == NULL)
+ errx(1, "could not allocate argv");
+
+ rumprun_cmdline_argc = argc;
+
+ argv[argc--] = NULL;
+ while (argc >= 0) {
+ argv[argc] = token2cstr(t + argc, data);
+ argc--;
+ }
+
rumprun_cmdline_argv = argv;
- rumprun_cmdline_argc = nargs;
}
static int
handle_cmdline(jsmntok_t *t, int left, char *data)
{
- T_CHECKTYPE(t, data, JSMN_STRING, __func__);
+ T_CHECKTYPE(t, data, JSMN_ARRAY, __func__);
+ /* discard the array token */
+ left--;
+ t++;
- makeargv(token2cstr(t, data));
+ jsmntok_t *const argv_tokens = t;
+ /* if the next token parent changes, we are done parsing argv array */
+ const int parent = t->parent;
+ int argc = 0;
- return 1;
+ while (argc < left && t->parent == parent) {
+ /* this array mimics argv, it must only contain strings */
+ T_CHECKTYPE(t, data, JSMN_STRING, __func__);
+ argc++;
+ t++;
+ }
+
+ makeargv(argv_tokens, data, argc);
+ return 1 + argc; /* array token + number of tokens inside the array */
}
static int
@@ -564,7 +597,7 @@ _rumprun_config(char *cmdline)
while (*cmdline != '{') {
if (*cmdline == '\0') {
warnx("could not find start of json. no config?");
- makeargv("rumprun");
+ makedefaultargv();
return;
}
cmdline++;
diff --git a/lib/librumprun_tester/rumprun_tester.c
b/lib/librumprun_tester/rumprun_tester.c
index 364ffdd..3644901 100644
--- a/lib/librumprun_tester/rumprun_tester.c
+++ b/lib/librumprun_tester/rumprun_tester.c
@@ -104,7 +104,7 @@ main(int argc, char *argv[])
*/
printf("=== FOE RUMPRUN 12345 TES-TER 54321 ===\n");
atexit(logexit);
- logrv = rumprun_test(argc-1, argv+1);
+ logrv = rumprun_test(argc, argv);
printf("=== RUMPRUN 12345 TES-TER 54321 EOF ===\n");
exit(logrv);
diff --git a/tests/Makefile b/tests/Makefile
index a262492..694a378 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -3,10 +3,12 @@ all-tests:
$(MAKE) -C hello
$(MAKE) -C basic
$(MAKE) -C crypto
+ $(MAKE) -C shell\ dir
.PHONY: clean
clean:
$(MAKE) -C hello clean
$(MAKE) -C basic clean
$(MAKE) -C crypto clean
+ $(MAKE) -C shell\ dir clean
[ ! -f configure/Makefile ] || $(MAKE) -C configure distclean
diff --git a/tests/runtests.sh b/tests/runtests.sh
index 9e06b5d..2ec33c1 100755
--- a/tests/runtests.sh
+++ b/tests/runtests.sh
@@ -33,9 +33,16 @@ RUMPSTOP=$(pwd)/../app-tools/rumpstop
export RUMPRUN_WARNING_STFU=please
# TODO: use a more scalable way of specifying tests
-TESTS='hello/hello.bin basic/ctor_test.bin basic/pthread_test.bin
- basic/tls_test.bin basic/misc_test.bin crypto/md5.bin'
-[ -x hello/hellopp.bin ] && TESTS="${TESTS} hello/hellopp.bin"
+TESTS="hello/hello.bin
+basic/ctor_test.bin
+basic/pthread_test.bin
+basic/tls_test.bin
+basic/misc_test.bin
+crypto/md5.bin
+shell dir/shell_check.bin
+shell dir/;'\"escaping\\\"'\" \" test.bin"
+[ -x hello/hellopp.bin ] && TESTS="${TESTS}
+hello/hellopp.bin"
STARTMAGIC='=== FOE RUMPRUN 12345 TES-TER 54321 ==='
ENDMAGIC='=== RUMPRUN 12345 TES-TER 54321 EOF ==='
@@ -62,7 +69,7 @@ ddimage ()
[ ${imgsize} -eq $((${blocks}*512)) ] \
|| die imgsize \"${imgsize}\" not 512-byte aligned
- dd if=${imgsource} of=${imgname} bs=512 count=${blocks} > /dev/null 2>&1
+ dd if="${imgsource}" of="${imgname}" bs=512 count=${blocks} > /dev/null
2>&1
}
runguest ()
@@ -74,7 +81,7 @@ runguest ()
# img2=$3
[ -n "${img1}" ] || die runtest without a disk image
- cookie=$(${RUMPRUN} ${OPT_SUDO} ${STACK} -b ${img1} ${testprog} __test)
+ cookie=$(${RUMPRUN} ${OPT_SUDO} ${STACK} "${testprog}" -b "${img1}"
__test)
if [ $? -ne 0 -o -z "${cookie}" ]; then
TEST_RESULT=ERROR
TEST_ECODE=-2
@@ -84,7 +91,7 @@ runguest ()
for x in $(seq 10) ; do
echo ">> polling, round ${x} ..."
- set -- $(sed 1q < ${img1})
+ set -- $(sed 1q < "${img1}")
case ${1} in
OK)
@@ -116,7 +123,7 @@ getoutput ()
img=${1}
shift || die 'getoutput: not enough args'
- sed -n "/${STARTMAGIC}/,/${ENDMAGIC}/p" < ${img} | sed -n '1n;$n;p'
+ sed -n "/${STARTMAGIC}/,/${ENDMAGIC}/p" < "${img}" | sed -n '1n;$n;p'
}
runtest ()
@@ -142,26 +149,28 @@ TESTDIR=$(mktemp -d testrun.XXXXXX)
[ $? -eq 0 ] || die failed to create datadir for testrun
TOPDIR=$(pwd)
-cd ${TESTDIR}
+cd "${TESTDIR}"
rv=0
-for test in ${TESTS}; do
+while read -r test; do
echo ">> Running test: ${test}"
- testunder="$(echo ${test} | sed s,/,_,g)"
+ testunder=$(echo "${test}" | sed s,/,_,g)
outputimg=${testunder}.disk1
- ddimage ${outputimg} $((2*512))
- runguest ${TOPDIR}/${test} ${outputimg}
+ ddimage "${outputimg}" $((2*512))
+ runguest "${TOPDIR}/${test}" "${outputimg}"
echo ">> Test output for ${test}"
- getoutput ${outputimg}
+ getoutput "${outputimg}"
echo ">> End test outout"
- echo ${test} ${TEST_RESULT} ${TEST_ECODE} >> test.log
+ echo "${test}" "${TEST_RESULT}" "${TEST_ECODE}" >> test.log
[ "${TEST_RESULT}" != 'SUCCESS' ] && rv=1
echo
-done
+done << EOF
+${TESTS}
+EOF
echo '>> TEST LOG'
cat test.log
diff --git a/tests/shell dir/Makefile b/tests/shell dir/Makefile
new file mode 100644
index 0000000..880593a
--- /dev/null
+++ b/tests/shell dir/Makefile
@@ -0,0 +1,9 @@
+include ../Makefile.inc
+
+# it seems gnu make can't handle filenames with spaces...
+ALL=shell_check.bin
+
+all: $(ALL)
+ cp shell_check.bin \;\'\"escaping\\\"\'\"\ \"\ \ test.bin
+clean:
+ rm -f \;\'\"escaping\\\"\'\"\ \"\ \ test.bin
diff --git a/tests/shell dir/shell_check.c b/tests/shell dir/shell_check.c
new file mode 100644
index 0000000..3e07679
--- /dev/null
+++ b/tests/shell dir/shell_check.c
@@ -0,0 +1,33 @@
+#include <stdio.h>
+#include <string.h>
+#include <libgen.h>
+#include <rumprun/tester.h>
+
+static char *expected_results[] = {
+ /* simple example */
+ "shell_check.bin",
+ /* more complex one for testing rumprun, json escaping seems incorrect:
+ should be "'\"escaping\\\"'\" \" test.bin" */
+ ";'\\\"escaping\\\"'\\\" \\\" test.bin",
+ NULL,
+};
+
+int
+rumprun_test(int argc, char *argv[])
+{
+ char *name = basename(*argv);
+ int i;
+
+ /* for each expected result */
+ for (i = 0; expected_results[i]; i++) {
+ /* test if argv[0] is in expected results */
+ if (!strcmp(name, expected_results[i])) {
+ break;
+ }
+ }
+ if (!expected_results[i]) {
+ printf("%s not found in the list of expected results!\n", name);
+ return 1;
+ }
+ return 0;
+}
