Re: [Qemu-devel] [PATCH 2/3] tests/cdboot: Test booting from CD-ROM ISO image file

[Eric Blake]

On 03/15/2018 05:48 AM, Thomas Huth wrote:

> > > +    pid = fork();
> > > +    if (pid == 0) {
> > > +        va_start(args, fmt);
> > > +        params = g_strdup_vprintf(fmt, args);
> > > +        va_end(args);
> > > +        command = g_strdup_printf("exec genisoimage %s", params);
> > > +        g_free(params);
> > > +        execlp("/bin/sh", "sh", "-c", command, NULL);
> > > +        exit(1);
> > > +    }
> > > +    wait(&status);
> >
> > IMHO this should just use g_spawn_sync(), also the use of
> > shell seems rather unneccessary

and potentially dangerous - if we aren't absolutely positive that we 
aren't going to improperly expand shell metacharacters embedded in params.

> > - why not just run genisoimage
> > directly ?
>
> That code was simply "inspired" from the execlp() code in
> qtest_init_without_qmp_handshake()

Sounds like a good idea for a future cleanup patch ;)

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org