[PATCH] kernel-shark: Provide parsing for quotation marks in Record command line

[PATCH] kernel-shark: Provide parsing for quotation marks in Record command line

[Yordan Karadzhov (VMware)]

Shell-like parsing of quotation marks in the content of the "Command"
field of the "Record" dialog will give more options to the users.
For example, now we can trace

 python -c 'print("hello world")'

Suggested-by: Stephen Brennan <stephen@brennan.io>
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204679
Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 kernel-shark/src/KsCaptureDialog.cpp | 38 ++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/kernel-shark/src/KsCaptureDialog.cpp b/kernel-shark/src/KsCaptureDialog.cpp
index dc1e9b2..b3393b6 100644
--- a/kernel-shark/src/KsCaptureDialog.cpp
+++ b/kernel-shark/src/KsCaptureDialog.cpp
@@ -157,7 +157,9 @@ KsCaptureControl::KsCaptureControl(QWidget *parent)
  */
 QStringList KsCaptureControl::getArgs()
 {
+	QString::SplitBehavior opt = QString::SkipEmptyParts;
 	QStringList argv;
+	QString cmdTmp;
 
 	argv << "record";
 
@@ -170,7 +172,36 @@ QStringList KsCaptureControl::getArgs()
 		argv << _eventsWidget.getCheckedEvents(true);
 
 	argv << "-o" << outputFileName();
-	argv << _commandLineEdit.text().split(" ");
+
+	cmdTmp = _commandLineEdit.text();
+	if (!cmdTmp.contains('\'') && !cmdTmp.contains('\"')) {
+		/* Split all command line arguments. */
+		argv << cmdTmp.split(" ", opt);
+	} else {
+		int iOpenQuots, iCloseQuots, size = cmdTmp.size();
+		int iSingleQuots = (cmdTmp.count('\'') == 2) ? cmdTmp.indexOf('\'') : size;
+		int iDoubleQuots = (cmdTmp.count('\"') == 2) ? cmdTmp.indexOf('\"') : size;
+
+		if (iSingleQuots < iDoubleQuots) {
+			iOpenQuots = iSingleQuots;
+			iCloseQuots = cmdTmp.lastIndexOf('\'') + 1;
+		} else if (iDoubleQuots < iSingleQuots) {
+			iOpenQuots = iDoubleQuots;
+			iCloseQuots = cmdTmp.lastIndexOf('\"') + 1;
+		} else {
+			emit print("\nERROR: Unable to parse the command.");
+			return {};
+		}
+
+		/* Split the arguments. */
+		argv << cmdTmp.left(iOpenQuots).split(" ", opt);
+
+		/* Everything in between the quotation marks goes in one piece. */
+		argv << cmdTmp.mid(iOpenQuots, iCloseQuots - iOpenQuots);
+
+		/* Split the rest of the arguments. */
+		argv << cmdTmp.right(size - iCloseQuots).split(" ", opt);
+	}
 
 	return argv;
 }
@@ -350,7 +381,10 @@ void KsCaptureControl::_browse()
 
 void KsCaptureControl::_apply()
 {
-	emit argsReady(getArgs().join(" "));
+	QStringList argv = getArgs();
+
+	if (argv.count())
+		emit argsReady(argv.join(" "));
 }
 
 /** @brief Create KsCaptureMonitor widget. */
-- 
2.20.1

Re: [PATCH] kernel-shark: Provide parsing for quotation marks in Record command line

[stephen]

Hi Yordan!

Thanks for the quick work on this issue :)

On Tue, Aug 27 16:14, Yordan Karadzhov (VMware) wrote:
> Shell-like parsing of quotation marks in the content of the "Command"
> field of the "Record" dialog will give more options to the users.
> For example, now we can trace
> 
>  python -c 'print("hello world")'
> 
> Suggested-by: Stephen Brennan <stephen@brennan.io>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204679
> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
> ---
>  kernel-shark/src/KsCaptureDialog.cpp | 38 ++++++++++++++++++++++++++--
>  1 file changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel-shark/src/KsCaptureDialog.cpp b/kernel-shark/src/KsCaptureDialog.cpp
> index dc1e9b2..b3393b6 100644
> --- a/kernel-shark/src/KsCaptureDialog.cpp
> +++ b/kernel-shark/src/KsCaptureDialog.cpp
> @@ -157,7 +157,9 @@ KsCaptureControl::KsCaptureControl(QWidget *parent)
>   */
>  QStringList KsCaptureControl::getArgs()
>  {
> +	QString::SplitBehavior opt = QString::SkipEmptyParts;
>  	QStringList argv;
> +	QString cmdTmp;
>  
>  	argv << "record";
>  
> @@ -170,7 +172,36 @@ QStringList KsCaptureControl::getArgs()
>  		argv << _eventsWidget.getCheckedEvents(true);
>  
>  	argv << "-o" << outputFileName();
> -	argv << _commandLineEdit.text().split(" ");
> +
> +	cmdTmp = _commandLineEdit.text();
> +	if (!cmdTmp.contains('\'') && !cmdTmp.contains('\"')) {
> +		/* Split all command line arguments. */
> +		argv << cmdTmp.split(" ", opt);
> +	} else {
> +		int iOpenQuots, iCloseQuots, size = cmdTmp.size();
> +		int iSingleQuots = (cmdTmp.count('\'') == 2) ? cmdTmp.indexOf('\'') : size;
> +		int iDoubleQuots = (cmdTmp.count('\"') == 2) ? cmdTmp.indexOf('\"') : size;
> +
> +		if (iSingleQuots < iDoubleQuots) {
> +			iOpenQuots = iSingleQuots;
> +			iCloseQuots = cmdTmp.lastIndexOf('\'') + 1;
> +		} else if (iDoubleQuots < iSingleQuots) {
> +			iOpenQuots = iDoubleQuots;
> +			iCloseQuots = cmdTmp.lastIndexOf('\"') + 1;
> +		} else {
> +			emit print("\nERROR: Unable to parse the command.");
> +			return {};
> +		}
> +
> +		/* Split the arguments. */
> +		argv << cmdTmp.left(iOpenQuots).split(" ", opt);
> +
> +		/* Everything in between the quotation marks goes in one piece. */
> +		argv << cmdTmp.mid(iOpenQuots, iCloseQuots - iOpenQuots);
> +
> +		/* Split the rest of the arguments. */
> +		argv << cmdTmp.right(size - iCloseQuots).split(" ", opt);
> +	}

It strikes me that this explicitly supports only a single set of quotes.
This sort of behavior would be pretty surprising for people expecting shell
quote support, and for people expecting just splitting on spaces.

I looked and couldn't really find any Qt utility for properly parsing shell
quoting (similar to python's shlex module). I totally get that it's a lot
of work to implement a correct shell quoting parser.

Maybe a compromise would be to add a checkbox to the capture dialog, which
tells kernel-shark to pass the entire textbox contents, unmodified, to the
shell implementation on the system. So, my example of:

    python -c 'print("hello world")'

Would get put into the third argument of the command:

    /bin/sh -c INSERT_TEXTBOX_CONTENTS_HERE

Then you could rely on /bin/sh doing the parsing for you. The downside is
that it adds a whole new process. But you can't always get everything in
life, right?

Hope this was helpful!

Stephen

Re: [PATCH] kernel-shark: Provide parsing for quotation marks in Record command line

[Steven Rostedt]

On Tue, 27 Aug 2019 22:10:18 -0700
stephen@brennan.io wrote:

> It strikes me that this explicitly supports only a single set of quotes.
> This sort of behavior would be pretty surprising for people expecting shell
> quote support, and for people expecting just splitting on spaces.
> 
> I looked and couldn't really find any Qt utility for properly parsing shell
> quoting (similar to python's shlex module). I totally get that it's a lot
> of work to implement a correct shell quoting parser.
> 
> Maybe a compromise would be to add a checkbox to the capture dialog, which
> tells kernel-shark to pass the entire textbox contents, unmodified, to the
> shell implementation on the system. So, my example of:
> 
>     python -c 'print("hello world")'
> 
> Would get put into the third argument of the command:
> 
>     /bin/sh -c INSERT_TEXTBOX_CONTENTS_HERE
> 
> Then you could rely on /bin/sh doing the parsing for you. The downside is
> that it adds a whole new process. But you can't always get everything in
> life, right?
> 

I need to look at this a bit deeper. I've written lots of cases where I
had to capture single and double quotes and turn them into a single
command. This is definitely needed here.

I'm very reluctant to just use a simple /bin/sh, as this is being run
as root. Grant you, it is open to do anything, but I rather not just
make it into a root shell. Although we still allow you to run any
command. But once you add a full shell with the "sh -c" you now need to
deal with environment variables and such, which can cause more
unexpected side effects.

Basically, we want to be able to add single and double quotes, as well
as backslashes: my -c 'command\'s here' and '\\' this "too"

Thanks for the feedback.

-- Steve

Re: [PATCH] kernel-shark: Provide parsing for quotation marks in Record command line

[Yordan Karadzhov (VMware)]

On 28.08.19 г. 16:06 ч., Steven Rostedt wrote:
> On Tue, 27 Aug 2019 22:10:18 -0700
> stephen@brennan.io wrote:
> 
>> It strikes me that this explicitly supports only a single set of quotes.
>> This sort of behavior would be pretty surprising for people expecting shell
>> quote support, and for people expecting just splitting on spaces.
>>
>> I looked and couldn't really find any Qt utility for properly parsing shell
>> quoting (similar to python's shlex module). I totally get that it's a lot
>> of work to implement a correct shell quoting parser.
>>
>> Maybe a compromise would be to add a checkbox to the capture dialog, which
>> tells kernel-shark to pass the entire textbox contents, unmodified, to the
>> shell implementation on the system. So, my example of:
>>
>>      python -c 'print("hello world")'
>>
>> Would get put into the third argument of the command:
>>
>>      /bin/sh -c INSERT_TEXTBOX_CONTENTS_HERE
>>
>> Then you could rely on /bin/sh doing the parsing for you. The downside is
>> that it adds a whole new process. But you can't always get everything in
>> life, right?
>>
> 
> I need to look at this a bit deeper. I've written lots of cases where I
> had to capture single and double quotes and turn them into a single
> command. This is definitely needed here.
> 

Can you send me few of the of most trickier examples that comes to your 
mind, so that I can use them to test the parsing?

Thanks!
Yordan

> I'm very reluctant to just use a simple /bin/sh, as this is being run
> as root. Grant you, it is open to do anything, but I rather not just
> make it into a root shell. Although we still allow you to run any
> command. But once you add a full shell with the "sh -c" you now need to
> deal with environment variables and such, which can cause more
> unexpected side effects.
> 
> Basically, we want to be able to add single and double quotes, as well
> as backslashes: my -c 'command\'s here' and '\\' this "too"
> 
> Thanks for the feedback.
> 
> -- Steve
> 

Re: [PATCH] kernel-shark: Provide parsing for quotation marks in Record command line

[Steven Rostedt]

On Wed, 28 Aug 2019 17:10:48 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> On 28.08.19 г. 16:06 ч., Steven Rostedt wrote:
> > On Tue, 27 Aug 2019 22:10:18 -0700
> > stephen@brennan.io wrote:
> >   
> >> It strikes me that this explicitly supports only a single set of quotes.
> >> This sort of behavior would be pretty surprising for people expecting shell
> >> quote support, and for people expecting just splitting on spaces.
> >>
> >> I looked and couldn't really find any Qt utility for properly parsing shell
> >> quoting (similar to python's shlex module). I totally get that it's a lot
> >> of work to implement a correct shell quoting parser.
> >>
> >> Maybe a compromise would be to add a checkbox to the capture dialog, which
> >> tells kernel-shark to pass the entire textbox contents, unmodified, to the
> >> shell implementation on the system. So, my example of:
> >>
> >>      python -c 'print("hello world")'
> >>
> >> Would get put into the third argument of the command:
> >>
> >>      /bin/sh -c INSERT_TEXTBOX_CONTENTS_HERE
> >>
> >> Then you could rely on /bin/sh doing the parsing for you. The downside is
> >> that it adds a whole new process. But you can't always get everything in
> >> life, right?
> >>  
> > 
> > I need to look at this a bit deeper. I've written lots of cases where I
> > had to capture single and double quotes and turn them into a single
> > command. This is definitely needed here.
> >   
> 
> Can you send me few of the of most trickier examples that comes to your 
> mind, so that I can use them to test the parsing?
> 

I was thinking of when we start adding synthetic event code, we will
definitely need to handle some of this. Like:

 --select 'irq_lat: lat=sched_waking.common_timestamp.usecs - hrtimer_start.common_timestamp.usecs, pid=sched_waking.pid
                    from timer.hrtimer_start join sched.sched_waking
                      on hrtimer_start.common_pid == sched_waking.common_pid
		    where hrtimer_start.function == 0xffffffff81200580 and
                          sched_waking.common_flags & 1'

 --select "wake_lat: lat=sched_switch.common_timestamp.usecs - irq_lat.common_timestamp.usecs, irqlat=irq_lat.lat, pid=sched_switch.next_pid
             from sched.sched_switch join irq_lat on sched_switch.next_pid == irq_lat.pip where next_comm == \"my-rt-code\""


Note, the above will be all on one single line!

-- Steve

Re: [PATCH] kernel-shark: Provide parsing for quotation marks in Record command line

[Steven Rostedt]

On Tue, 27 Aug 2019 16:14:18 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> +	cmdTmp = _commandLineEdit.text();
> +	if (!cmdTmp.contains('\'') && !cmdTmp.contains('\"')) {
> +		/* Split all command line arguments. */
> +		argv << cmdTmp.split(" ", opt);
> +	} else {
> +		int iOpenQuots, iCloseQuots, size = cmdTmp.size();
> +		int iSingleQuots = (cmdTmp.count('\'') == 2) ? cmdTmp.indexOf('\'') : size;
> +		int iDoubleQuots = (cmdTmp.count('\"') == 2) ? cmdTmp.indexOf('\"') : size;
> +
> +		if (iSingleQuots < iDoubleQuots) {
> +			iOpenQuots = iSingleQuots;
> +			iCloseQuots = cmdTmp.lastIndexOf('\'') + 1;
> +		} else if (iDoubleQuots < iSingleQuots) {
> +			iOpenQuots = iDoubleQuots;
> +			iCloseQuots = cmdTmp.lastIndexOf('\"') + 1;
> +		} else {
> +			emit print("\nERROR: Unable to parse the command.");
> +			return {};
> +		}
> +
> +		/* Split the arguments. */
> +		argv << cmdTmp.left(iOpenQuots).split(" ", opt);
> +
> +		/* Everything in between the quotation marks goes in one piece. */
> +		argv << cmdTmp.mid(iOpenQuots, iCloseQuots - iOpenQuots);
> +
> +		/* Split the rest of the arguments. */
> +		argv << cmdTmp.right(size - iCloseQuots).split(" ", opt);
> +	}

This is where I hate C++, because it makes simple things so
complicated ;-)

What we need to do is simply:

	char *str = _commandLineEdit.text();
	char *last_word = str;
	char quote = 0;
	int i;

	// remove front and end spaces
	while (isspace(*str))
		str++;
	i = strlen(str);
	while (i > 0 && isspace(str[i-1])
		i--;
	str[i] = 0;

	for (i = 0; str[i]; i++) {
		if (isspace(str[i]) && !quote) {
			str[i++] = 0;
			argv << last_word;
			while (isspace(str[i]))
				i++;
			last_word = &str[i];
		}
		switch(str[i]) {
		case '\\':
			i++;
			break;
		case '\'':
		case '"':
			if (!quote)
				quote = str[i];
			else if (quote == str[i])
				quote = 0;
			break;
		}
	}
	argv << last_word;

Note, the above may be buggy, I didn't test it.

-- Steve

Re: [PATCH] kernel-shark: Provide parsing for quotation marks in Record command line

[Steven Rostedt]

On Wed, 28 Aug 2019 09:25:33 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Tue, 27 Aug 2019 16:14:18 +0300
> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> 
> > +	cmdTmp = _commandLineEdit.text();
> > +	if (!cmdTmp.contains('\'') && !cmdTmp.contains('\"')) {
> > +		/* Split all command line arguments. */
> > +		argv << cmdTmp.split(" ", opt);
> > +	} else {
> > +		int iOpenQuots, iCloseQuots, size = cmdTmp.size();
> > +		int iSingleQuots = (cmdTmp.count('\'') == 2) ? cmdTmp.indexOf('\'') : size;
> > +		int iDoubleQuots = (cmdTmp.count('\"') == 2) ? cmdTmp.indexOf('\"') : size;
> > +
> > +		if (iSingleQuots < iDoubleQuots) {
> > +			iOpenQuots = iSingleQuots;
> > +			iCloseQuots = cmdTmp.lastIndexOf('\'') + 1;
> > +		} else if (iDoubleQuots < iSingleQuots) {
> > +			iOpenQuots = iDoubleQuots;
> > +			iCloseQuots = cmdTmp.lastIndexOf('\"') + 1;
> > +		} else {
> > +			emit print("\nERROR: Unable to parse the command.");
> > +			return {};
> > +		}
> > +
> > +		/* Split the arguments. */
> > +		argv << cmdTmp.left(iOpenQuots).split(" ", opt);
> > +
> > +		/* Everything in between the quotation marks goes in one piece. */
> > +		argv << cmdTmp.mid(iOpenQuots, iCloseQuots - iOpenQuots);
> > +
> > +		/* Split the rest of the arguments. */
> > +		argv << cmdTmp.right(size - iCloseQuots).split(" ", opt);
> > +	}  
> 
> This is where I hate C++, because it makes simple things so
> complicated ;-)
> 
> What we need to do is simply:
> 
> 	char *str = _commandLineEdit.text();
> 	char *last_word = str;
> 	char quote = 0;
> 	int i;
> 
> 	// remove front and end spaces
> 	while (isspace(*str))
> 		str++;
> 	i = strlen(str);
> 	while (i > 0 && isspace(str[i-1])
> 		i--;
> 	str[i] = 0;

Oops, need to have:

	last_word = &str[i];

here.

-- Steve

> 
> 	for (i = 0; str[i]; i++) {
> 		if (isspace(str[i]) && !quote) {
> 			str[i++] = 0;
> 			argv << last_word;
> 			while (isspace(str[i]))
> 				i++;
> 			last_word = &str[i];
> 		}
> 		switch(str[i]) {
> 		case '\\':
> 			i++;
> 			break;
> 		case '\'':
> 		case '"':
> 			if (!quote)
> 				quote = str[i];
> 			else if (quote == str[i])
> 				quote = 0;
> 			break;
> 		}
> 	}
> 	argv << last_word;
> 
> Note, the above may be buggy, I didn't test it.
> 
> -- Steve

Re: [PATCH] kernel-shark: Provide parsing for quotation marks in Record command line

[Yordan Karadzhov (VMware)]

On 28.08.19 г. 14:30 ч., Steven Rostedt wrote:
> On Wed, 28 Aug 2019 09:25:33 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
>> On Tue, 27 Aug 2019 16:14:18 +0300
>> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
>>
>>> +	cmdTmp = _commandLineEdit.text();
>>> +	if (!cmdTmp.contains('\'') && !cmdTmp.contains('\"')) {
>>> +		/* Split all command line arguments. */
>>> +		argv << cmdTmp.split(" ", opt);
>>> +	} else {
>>> +		int iOpenQuots, iCloseQuots, size = cmdTmp.size();
>>> +		int iSingleQuots = (cmdTmp.count('\'') == 2) ? cmdTmp.indexOf('\'') : size;
>>> +		int iDoubleQuots = (cmdTmp.count('\"') == 2) ? cmdTmp.indexOf('\"') : size;
>>> +
>>> +		if (iSingleQuots < iDoubleQuots) {
>>> +			iOpenQuots = iSingleQuots;
>>> +			iCloseQuots = cmdTmp.lastIndexOf('\'') + 1;
>>> +		} else if (iDoubleQuots < iSingleQuots) {
>>> +			iOpenQuots = iDoubleQuots;
>>> +			iCloseQuots = cmdTmp.lastIndexOf('\"') + 1;
>>> +		} else {
>>> +			emit print("\nERROR: Unable to parse the command.");
>>> +			return {};
>>> +		}
>>> +
>>> +		/* Split the arguments. */
>>> +		argv << cmdTmp.left(iOpenQuots).split(" ", opt);
>>> +
>>> +		/* Everything in between the quotation marks goes in one piece. */
>>> +		argv << cmdTmp.mid(iOpenQuots, iCloseQuots - iOpenQuots);
>>> +
>>> +		/* Split the rest of the arguments. */
>>> +		argv << cmdTmp.right(size - iCloseQuots).split(" ", opt);
>>> +	}
>>
>> This is where I hate C++, because it makes simple things so
>> complicated ;-)
>>
>> What we need to do is simply:
>>
>> 	char *str = _commandLineEdit.text();
>> 	char *last_word = str;
>> 	char quote = 0;
>> 	int i;
>>
>> 	// remove front and end spaces
>> 	while (isspace(*str))
>> 		str++;
>> 	i = strlen(str);
>> 	while (i > 0 && isspace(str[i-1])
>> 		i--;
>> 	str[i] = 0;
> 
> Oops, need to have:
> 
> 	last_word = &str[i];
> 
> here.
> 
> -- Steve
> 
>>
>> 	for (i = 0; str[i]; i++) {
>> 		if (isspace(str[i]) && !quote) {
>> 			str[i++] = 0;
>> 			argv << last_word;
>> 			while (isspace(str[i]))
>> 				i++;
>> 			last_word = &str[i];
>> 		}
>> 		switch(str[i]) {
>> 		case '\\':
>> 			i++;
>> 			break;
>> 		case '\'':
>> 		case '"':
>> 			if (!quote)
>> 				quote = str[i];
>> 			else if (quote == str[i])
>> 				quote = 0;
>> 			break;
>> 		}
>> 	}
>> 	argv << last_word;
>>
>> Note, the above may be buggy, I didn't test it.
>>

Hi Steve,

I understand very well your feelings ;)
However, I also really dislike eclectic programming stiles. Since this 
is part of the C++/Qt code, I would prefer to stick to the C++/Qt way of 
doing things.

Here is a version of your code that is more C++/Qt-ish:


	QString::SplitBehavior opt = QString::SkipEmptyParts;
	QChar quote = 0;
	int i, progress = 0, size;

	cmd.remove(QChar('\\'));
	size = cmd.count();
	auto lamMid = [&] () {return cmd.mid(progress, i - progress);};
	for (i = 0; i < size; ++i) {
		if (cmd[i] == '\'' || cmd[i] == '"') {
			if (quote.isNull()) {
				args << lamMid().split(" ", opt);
				quote = cmd[i++];
				progress = i;
			} else if (quote == cmd[i]) {
				args << lamMid();
				quote = 0;
				progress = ++i;
			}
		}
	}

	args << cmd.right(size - progress).split(" ", opt);

If you are OK with this I will send a new version of the patch.

Thanks!
Yordan



>> -- Steve
> 

Re: [PATCH] kernel-shark: Provide parsing for quotation marks in Record command line

[Steven Rostedt]

On Tue, 17 Sep 2019 13:44:52 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> >> This is where I hate C++, because it makes simple things so
> >> complicated ;-)
> >>
> >> What we need to do is simply:
> >>
> >> 	char *str = _commandLineEdit.text();
> >> 	char *last_word = str;
> >> 	char quote = 0;
> >> 	int i;
> >>
> >> 	// remove front and end spaces
> >> 	while (isspace(*str))
> >> 		str++;
> >> 	i = strlen(str);
> >> 	while (i > 0 && isspace(str[i-1])
> >> 		i--;
> >> 	str[i] = 0;  
> > 
> > Oops, need to have:
> > 
> > 	last_word = &str[i];
> > 
> > here.
> > 
> > -- Steve
> >   
> >>
> >> 	for (i = 0; str[i]; i++) {
> >> 		if (isspace(str[i]) && !quote) {
> >> 			str[i++] = 0;
> >> 			argv << last_word;
> >> 			while (isspace(str[i]))
> >> 				i++;
> >> 			last_word = &str[i];
> >> 		}
> >> 		switch(str[i]) {
> >> 		case '\\':
> >> 			i++;
> >> 			break;
> >> 		case '\'':
> >> 		case '"':
> >> 			if (!quote)
> >> 				quote = str[i];
> >> 			else if (quote == str[i])
> >> 				quote = 0;
> >> 			break;
> >> 		}
> >> 	}
> >> 	argv << last_word;
> >>
> >> Note, the above may be buggy, I didn't test it.
> >>  
> 
> Hi Steve,
> 
> I understand very well your feelings ;)
> However, I also really dislike eclectic programming stiles. Since this 
> is part of the C++/Qt code, I would prefer to stick to the C++/Qt way of 
> doing things.

I'm fine with that as you will be maintaining it. But it also needs to
be correct.

> 
> Here is a version of your code that is more C++/Qt-ish:
> 
> 
> 	QString::SplitBehavior opt = QString::SkipEmptyParts;
> 	QChar quote = 0;
> 	int i, progress = 0, size;
> 
> 	cmd.remove(QChar('\\'));

What does the above do? Removes all backslashes? Why? A backslashed
quote must be ignored.

 '\'' is a single quote within quotes.

Not to mention, a backslash keeps whatever it backslashed:

 \$ == $
 \' == '
 \" == "
 \1 == 1
 \\ == \

I don't see how this code handles this, as my code does.

-- Steve


> 	size = cmd.count();
> 	auto lamMid = [&] () {return cmd.mid(progress, i - progress);};
> 	for (i = 0; i < size; ++i) {
> 		if (cmd[i] == '\'' || cmd[i] == '"') {
> 			if (quote.isNull()) {
> 				args << lamMid().split(" ", opt);
> 				quote = cmd[i++];
> 				progress = i;
> 			} else if (quote == cmd[i]) {
> 				args << lamMid();
> 				quote = 0;
> 				progress = ++i;
> 			}
> 		}
> 	}
> 
> 	args << cmd.right(size - progress).split(" ", opt);
> 
> If you are OK with this I will send a new version of the patch.
> 
> Thanks!
> Yordan
> 
> 
> 
> >> -- Steve  
> >