diff mbox series

[v2,1/3] kernel-shark: Provide parsing for quotation marks in Record command line

Message ID 20190918142319.11821-2-y.karadz@gmail.com (mailing list archive)
State Accepted
Headers show
Series Support "shell quoting" in the Record dialog | expand

Commit Message

Yordan Karadzhov Sept. 18, 2019, 2:23 p.m. UTC 
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")'

The patch also adds support for multi-line command options.

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 | 10 +++++--
 kernel-shark/src/KsUtils.cpp         | 41 ++++++++++++++++++++++++++++
 kernel-shark/src/KsUtils.hpp         |  2 ++
 3 files changed, 50 insertions(+), 3 deletions(-)

Comments

Steven Rostedt Sept. 19, 2019, 11:20 p.m. UTC | #1 
On Wed, 18 Sep 2019 17:23:17 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> +/**
> + * Separate the command line arguments inside the string taking into account
> + * possible shell quoting and new lines.
> + */
> +QStringList splitArguments(QString cmd)
> +{
> +	QString::SplitBehavior opt = QString::SkipEmptyParts;
> +	int i, progress = 0, size;
> +	QStringList argv;
> +	QChar quote = 0;
> +
> +	/* Remove all new lines. */
> +	cmd.replace("\\\n", " ");
> +
> +	size = cmd.count();
> +	auto lamMid = [&] () {return cmd.mid(progress, i - progress);};
> +	for (i = 0; i < size; ++i) {
> +		if (cmd[i] == '\\') {
> +			cmd.remove(i, 1);
> +			size --;
> +			continue;
> +		}
> +
> +		if (cmd[i] == '\'' || cmd[i] == '"') {
> +			if (quote.isNull()) {
> +				argv << lamMid().split(" ", opt);
> +				quote = cmd[i++];
> +				progress = i;
> +			} else if (quote == cmd[i]) {
> +				argv << lamMid();
> +				quote = 0;
> +				progress = ++i;
> +			}
> +		}
> +	}
> +
> +	argv << cmd.right(size - progress).split(" ", opt);
> +
> +	return argv;
> +}

I still find the above hard to read, but so be it ;-) 

Anyway, not quite yet. I just noticed that if I do:

  echo "this \" is \" a \"test"

The output has:

("echo", "this \" is \" a \"test")


We don't want to keep the backslash here. We want to remove it before
passing it as an argument.

The above should be:

("echo", "this " is " a "test")

(thinking that you put in the outside quotes. In other words, if I have:

 echo "hello \"there\""

We should break that up into: 
  arg0=echo
  arg1=hello "there"

-- Steve
Yordan Karadzhov Sept. 20, 2019, 9:48 a.m. UTC | #2 
On 20.09.19 г. 2:20 ч., Steven Rostedt wrote:
> On Wed, 18 Sep 2019 17:23:17 +0300
> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> 
>> +/**
>> + * Separate the command line arguments inside the string taking into account
>> + * possible shell quoting and new lines.
>> + */
>> +QStringList splitArguments(QString cmd)
>> +{
>> +	QString::SplitBehavior opt = QString::SkipEmptyParts;
>> +	int i, progress = 0, size;
>> +	QStringList argv;
>> +	QChar quote = 0;
>> +
>> +	/* Remove all new lines. */
>> +	cmd.replace("\\\n", " ");
>> +
>> +	size = cmd.count();
>> +	auto lamMid = [&] () {return cmd.mid(progress, i - progress);};
>> +	for (i = 0; i < size; ++i) {
>> +		if (cmd[i] == '\\') {
>> +			cmd.remove(i, 1);
>> +			size --;
>> +			continue;
>> +		}
>> +
>> +		if (cmd[i] == '\'' || cmd[i] == '"') {
>> +			if (quote.isNull()) {
>> +				argv << lamMid().split(" ", opt);
>> +				quote = cmd[i++];
>> +				progress = i;
>> +			} else if (quote == cmd[i]) {
>> +				argv << lamMid();
>> +				quote = 0;
>> +				progress = ++i;
>> +			}
>> +		}
>> +	}
>> +
>> +	argv << cmd.right(size - progress).split(" ", opt);
>> +
>> +	return argv;
>> +}
> 
> I still find the above hard to read, but so be it ;-)
> 
> Anyway, not quite yet. I just noticed that if I do:
> 
>    echo "this \" is \" a \"test"
> 
> The output has:
> 
> ("echo", "this \" is \" a \"test")
> 

Hi Steven,

This is a Qt printing artifact. The printout is trying to show clearly 
that this list contain only 2 strings. That is why it adds those 
backslashes. In the code of the example, try adding this:

                 if (ok) {
                         argList = KsUtils::splitArguments(text);
                         qInfo() << argList;
+                       for (auto a: argList)
+                               cout << a.toStdString() << endl;

                         proc.setProgram(argList.takeFirst());
                         proc.setArguments(argList);


and you will see what I mean.

Thanks a lot for being a tester!!!
Yordan

> 
> We don't want to keep the backslash here. We want to remove it before
> passing it as an argument.
> 
> The above should be:
> 
> ("echo", "this " is " a "test")
> 
> (thinking that you put in the outside quotes. In other words, if I have:
> 
>   echo "hello \"there\""
> 
> We should break that up into:
>    arg0=echo
>    arg1=hello "there"
> 
> -- Steve
>
diff mbox series

Patch

diff --git a/kernel-shark/src/KsCaptureDialog.cpp b/kernel-shark/src/KsCaptureDialog.cpp
index dc1e9b2..7c2ef46 100644
--- a/kernel-shark/src/KsCaptureDialog.cpp
+++ b/kernel-shark/src/KsCaptureDialog.cpp
@@ -170,7 +170,8 @@  QStringList KsCaptureControl::getArgs()
 		argv << _eventsWidget.getCheckedEvents(true);
 
 	argv << "-o" << outputFileName();
-	argv << _commandLineEdit.text().split(" ");
+
+	argv << KsUtils::splitArguments(_commandLineEdit.text());
 
 	return argv;
 }
@@ -350,7 +351,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. */
@@ -547,7 +551,7 @@  void KsCaptureDialog::_capture()
 	int argc;
 
 	if(_captureMon._argsModified) {
-		argv = _captureMon.text().split(" ");
+		argv = KsUtils::splitArguments(_captureMon.text());
 	} else {
 		argv = _captureCtrl.getArgs();
 	}
diff --git a/kernel-shark/src/KsUtils.cpp b/kernel-shark/src/KsUtils.cpp
index 58ce8c1..e99509f 100644
--- a/kernel-shark/src/KsUtils.cpp
+++ b/kernel-shark/src/KsUtils.cpp
@@ -257,6 +257,47 @@  QString getSaveFile(QWidget *parent,
 	return fileName;
 }
 
+/**
+ * Separate the command line arguments inside the string taking into account
+ * possible shell quoting and new lines.
+ */
+QStringList splitArguments(QString cmd)
+{
+	QString::SplitBehavior opt = QString::SkipEmptyParts;
+	int i, progress = 0, size;
+	QStringList argv;
+	QChar quote = 0;
+
+	/* Remove all new lines. */
+	cmd.replace("\\\n", " ");
+
+	size = cmd.count();
+	auto lamMid = [&] () {return cmd.mid(progress, i - progress);};
+	for (i = 0; i < size; ++i) {
+		if (cmd[i] == '\\') {
+			cmd.remove(i, 1);
+			size --;
+			continue;
+		}
+
+		if (cmd[i] == '\'' || cmd[i] == '"') {
+			if (quote.isNull()) {
+				argv << lamMid().split(" ", opt);
+				quote = cmd[i++];
+				progress = i;
+			} else if (quote == cmd[i]) {
+				argv << lamMid();
+				quote = 0;
+				progress = ++i;
+			}
+		}
+	}
+
+	argv << cmd.right(size - progress).split(" ", opt);
+
+	return argv;
+}
+
 }; // KsUtils
 
 /** A stream operator for converting QColor into KsPlot::Color. */
diff --git a/kernel-shark/src/KsUtils.hpp b/kernel-shark/src/KsUtils.hpp
index 7362c14..db1bf5e 100644
--- a/kernel-shark/src/KsUtils.hpp
+++ b/kernel-shark/src/KsUtils.hpp
@@ -130,6 +130,8 @@  QString getSaveFile(QWidget *parent,
 		    const QString &extension,
 		    QString &lastFilePath);
 
+QStringList splitArguments(QString cmd);
+
 }; // KsUtils
 
 /** Identifier of the Dual Marker active state. */