qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH] qemu-iotests: Fix _filter_qemu
Date: Tue, 16 Apr 2013 06:26:38 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130402 Thunderbird/17.0.5

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 <address@hidden>
> ---
>  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 <address@hidden>

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

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]