Stefan Hajnoczi <[email protected]> writes: > On Mon, Feb 17, 2014 at 05:44:55PM +0100, Markus Armbruster wrote: >> Stefan Hajnoczi <[email protected]> writes: >> >> > qtest_init() cannot use exec*p() to launch QEMU since the exec*p() >> > functions take an argument array while qtest_init() takes char >> > *extra_args. Therefore we execute /bin/sh -c <command-line> and let the >> > shell parse the argument string. >> > >> > This left /bin/sh as our child process and our child's child was QEMU. >> > We still want QEMU's pid so the -pidfile option was used to let QEMU >> > report its pid. >> > >> > The pidfile needs to be unlinked when the test case exits or fails. In >> > other words, the pidfile creates a new problem for us! >> > >> > Simplify all this using the shell 'exec' command. It allows us to >> > replace the /bin/sh process with QEMU. Then we no longer need to use >> > -pidfile because we already know our fork child's pid. >> > >> > Note: Yes, it seems silly to exec /bin/sh when we could just exec QEMU >> > directly. But remember qtest_init() takes a single char *extra_args >> > command-line fragment instead of a real argv[] array, so we need >> > /bin/sh's argument parsing behavior. >> >> Sounds like a design mistake to me. > > I wouldn't call char *extra_args a mistake because strings are still > simpler to manipulate in C than char *argv[] arrays. So we write less > code in test cases and libqtest.c at the expense of a roundabout way to > spawn the process.
Building command arguments in a string is deceptively simple. The deception collapses once you add funny characters that make the shell do funny and unexpected things. Our extra_args string manipulations are typically of the form "format a bunch of options and option arguments into a string". I suspect using a function to collect a bunch of options and option arguments into an array would be about the same amount of code in most cases. Just sayin'; I'm not objecting to your patch.
