qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Stefan Hajnoczi <stefanha@redhat.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] qemu-iotests: Fix _filter_qemu
Date: Tue, 16 Apr 2013 15:56:42 +0200	[thread overview]
Message-ID: <20130416135642.GC19758@dhcp-200-207.str.redhat.com> (raw)
In-Reply-To: <20130416131611.GA25650@stefanha-thinkpad.redhat.com>

Am 16.04.2013 um 15:16 hat Stefan Hajnoczi geschrieben:
> On Tue, Apr 16, 2013 at 06:26:38AM -0600, Eric Blake wrote:
> > 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.
> 
> sed -e is the convention in qemu-iotests.
> 
> Dropping the 'g' would be nice.

Okay, I just updated the commit to drop the 'g'.

> The problem with the POSIX shell string replacement is that the syntax
> is horrible.  I can never remember what ${%}, ${%%}, ${#} and
> %{##} do.  $(basename $QEMU_PROG) is clear (although it doesn't handle
> spaces in the filename!).

That's exactly my problem as well.

Kevin

  parent reply	other threads:[~2013-04-16 14:32 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-16  9:48 [Qemu-devel] [PATCH] qemu-iotests: Fix _filter_qemu Kevin Wolf
2013-04-16 12:26 ` Eric Blake
2013-04-16 13:16   ` Stefan Hajnoczi
2013-04-16 13:52     ` Markus Armbruster
2013-04-16 15:08       ` Stefan Hajnoczi
2013-04-16 13:56     ` Kevin Wolf [this message]
2013-04-16 14:08     ` Eric Blake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130416135642.GC19758@dhcp-200-207.str.redhat.com \
    --to=kwolf@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).