Re: [Qemu-devel] [PATCH] qemu-iotests: Fix _filter_qemu

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1438 bytes --]

On 04/16/2013 03:48 AM, Kevin Wolf wrote:
> $QEMU_PROG happens to be 'qemu' in my setup, so this sed command
> replaces a bit too much. Restrict it to the start of the line and to
> when it's followed by a colon, i.e. the form used by error messages.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  tests/qemu-iotests/common.filter | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
> index bc5f250..a7f889a 100644
> --- a/tests/qemu-iotests/common.filter
> +++ b/tests/qemu-iotests/common.filter
> @@ -155,7 +155,7 @@ _filter_qemu_io()
>  # replace occurrences of QEMU_PROG with "qemu"
>  _filter_qemu()
>  {
> -    sed -e "s#$(basename $QEMU_PROG)#QEMU_PROG#g"
> +    sed -e "s#^$(basename $QEMU_PROG):#QEMU_PROG:#g"

Why spawn a basename process, when you can use shell to do the same?
Also, the g modifier is worthless once you have a ^ anchor, since sed
can only replace one ^ per line.  And you don't need sed's -e option for
a single script.

The following is identical, with less typing:

_filter_qemu()
{
    sed "s#^${QEMU_PROG##*/}:#QEMU_PROG#"
}

That said, your more verbose version is still functionally correct, so
you can add:

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]