* [PATCH v2 1/3] kernel-shark: Provide parsing for quotation marks in Record command line
2019-09-18 14:23 [PATCH v2 0/3] Support "shell quoting" in the Record dialog Yordan Karadzhov (VMware)
@ 2019-09-18 14:23 ` Yordan Karadzhov (VMware)
2019-09-19 23:20 ` Steven Rostedt
2019-09-18 14:23 ` [PATCH v2 2/3] kernel-shark: give more space to the command field of the Record dialog Yordan Karadzhov (VMware)
2019-09-18 14:23 ` [PATCH v2 3/3] kernel-shark: Add quotation marks parsing example/test Yordan Karadzhov (VMware)
2 siblings, 1 reply; 8+ messages in thread
From: Yordan Karadzhov (VMware) @ 2019-09-18 14:23 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel, stephen, 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")'
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(-)
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. */
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/3] kernel-shark: Provide parsing for quotation marks in Record command line
2019-09-18 14:23 ` [PATCH v2 1/3] kernel-shark: Provide parsing for quotation marks in Record command line Yordan Karadzhov (VMware)
@ 2019-09-19 23:20 ` Steven Rostedt
2019-09-20 9:48 ` Yordan Karadzhov (VMware)
0 siblings, 1 reply; 8+ messages in thread
From: Steven Rostedt @ 2019-09-19 23:20 UTC (permalink / raw)
To: Yordan Karadzhov (VMware); +Cc: linux-trace-devel, stephen
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/3] kernel-shark: Provide parsing for quotation marks in Record command line
2019-09-19 23:20 ` Steven Rostedt
@ 2019-09-20 9:48 ` Yordan Karadzhov (VMware)
0 siblings, 0 replies; 8+ messages in thread
From: Yordan Karadzhov (VMware) @ 2019-09-20 9:48 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linux-trace-devel, stephen
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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/3] kernel-shark: give more space to the command field of the Record dialog
2019-09-18 14:23 [PATCH v2 0/3] Support "shell quoting" in the Record dialog Yordan Karadzhov (VMware)
2019-09-18 14:23 ` [PATCH v2 1/3] kernel-shark: Provide parsing for quotation marks in Record command line Yordan Karadzhov (VMware)
@ 2019-09-18 14:23 ` Yordan Karadzhov (VMware)
2019-09-18 14:23 ` [PATCH v2 3/3] kernel-shark: Add quotation marks parsing example/test Yordan Karadzhov (VMware)
2 siblings, 0 replies; 8+ messages in thread
From: Yordan Karadzhov (VMware) @ 2019-09-18 14:23 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel, stephen, Yordan Karadzhov (VMware)
Since the Record dialog not support parsing of complex multi-line
commands the corresponding field of the widget has to be able to
accommodate such multi-line user input.
Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
kernel-shark/src/KsCaptureDialog.cpp | 29 +++++++++++++++++++++++++---
kernel-shark/src/KsCaptureDialog.hpp | 18 ++++++++++++++++-
2 files changed, 43 insertions(+), 4 deletions(-)
diff --git a/kernel-shark/src/KsCaptureDialog.cpp b/kernel-shark/src/KsCaptureDialog.cpp
index 7c2ef46..fff42aa 100644
--- a/kernel-shark/src/KsCaptureDialog.cpp
+++ b/kernel-shark/src/KsCaptureDialog.cpp
@@ -29,6 +29,25 @@ static inline tep_handle *local_events()
return tracecmd_local_events(tracecmd_get_tracing_dir());
}
+/**
+ * @brief Create KsCommandLineEdit.
+ *
+ * @param text: Defaulst text to be shown.
+ * @param parent: The parent of this widget.
+ */
+KsCommandLineEdit::KsCommandLineEdit(QString text, QWidget *parent)
+: QPlainTextEdit(text, parent) {}
+
+QSize KsCommandLineEdit::minimumSizeHint() const
+{
+ return {FONT_WIDTH * 30, FONT_HEIGHT * 2};
+}
+
+QSize KsCommandLineEdit::sizeHint() const
+{
+ return {FONT_WIDTH * 30, FONT_HEIGHT * 3};
+}
+
/** @brief Create KsCaptureControl widget. */
KsCaptureControl::KsCaptureControl(QWidget *parent)
: QWidget(parent),
@@ -117,7 +136,11 @@ KsCaptureControl::KsCaptureControl(QWidget *parent)
_commandLabel.adjustSize();
_commandLabel.setFixedWidth(_outputLabel.width());
_execLayout.addWidget(&_commandLabel, row, 0);
+
_commandLineEdit.setFixedWidth(FONT_WIDTH * 30);
+ _commandLineEdit.setMinimumHeight(FONT_HEIGHT * 2);
+ _commandLineEdit.setMaximumHeight(FONT_HEIGHT * 9);
+
_execLayout.addWidget(&_commandLineEdit, row, 1);
_commandCheckBox.setCheckState(Qt::Unchecked);
_commandCheckBox.adjustSize();
@@ -171,7 +194,7 @@ QStringList KsCaptureControl::getArgs()
argv << "-o" << outputFileName();
- argv << KsUtils::splitArguments(_commandLineEdit.text());
+ argv << KsUtils::splitArguments(_commandLineEdit.toPlainText());
return argv;
}
@@ -282,7 +305,7 @@ void KsCaptureControl::_importSettings()
_outputLineEdit.setText(KS_C_STR_CAST(temp->conf_doc));
if (kshark_config_doc_get(conf, "Command", temp))
- _commandLineEdit.setText(KS_C_STR_CAST(temp->conf_doc));
+ _commandLineEdit.setPlainText(KS_C_STR_CAST(temp->conf_doc));
}
void KsCaptureControl::_exportSettings()
@@ -330,7 +353,7 @@ void KsCaptureControl::_exportSettings()
kshark_config_doc_add(conf, "Output", kshark_json_to_conf(jout));
/* Save the command. */
- comm = _commandLineEdit.text();
+ comm = _commandLineEdit.toPlainText();
json_object *jcomm = json_object_new_string(comm.toStdString().c_str());
kshark_config_doc_add(conf, "Command", kshark_json_to_conf(jcomm));
diff --git a/kernel-shark/src/KsCaptureDialog.hpp b/kernel-shark/src/KsCaptureDialog.hpp
index f8ddf4a..612080c 100644
--- a/kernel-shark/src/KsCaptureDialog.hpp
+++ b/kernel-shark/src/KsCaptureDialog.hpp
@@ -18,6 +18,20 @@
// KernelShark
#include "KsWidgetsLib.hpp"
+/**
+ * The KsCommandLineEdit class is used to override the default size hints of
+ * the QPlainTextEdit class.
+ */
+struct KsCommandLineEdit : public QPlainTextEdit
+{
+ KsCommandLineEdit(QString text, QWidget *parent = 0);
+
+private:
+ QSize sizeHint() const override;
+
+ QSize minimumSizeHint() const override;
+};
+
/**
* The KsCaptureControl class provides a control panel for the KernelShark
* Capture dialog.
@@ -57,7 +71,9 @@ private:
QLabel _pluginsLabel, _outputLabel, _commandLabel;
- QLineEdit _outputLineEdit, _commandLineEdit;
+ QLineEdit _outputLineEdit;
+
+ KsCommandLineEdit _commandLineEdit;
QToolBar _settingsToolBar, _controlToolBar;
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 3/3] kernel-shark: Add quotation marks parsing example/test
2019-09-18 14:23 [PATCH v2 0/3] Support "shell quoting" in the Record dialog Yordan Karadzhov (VMware)
2019-09-18 14:23 ` [PATCH v2 1/3] kernel-shark: Provide parsing for quotation marks in Record command line Yordan Karadzhov (VMware)
2019-09-18 14:23 ` [PATCH v2 2/3] kernel-shark: give more space to the command field of the Record dialog Yordan Karadzhov (VMware)
@ 2019-09-18 14:23 ` Yordan Karadzhov (VMware)
2019-09-19 23:12 ` Steven Rostedt
2 siblings, 1 reply; 8+ messages in thread
From: Yordan Karadzhov (VMware) @ 2019-09-18 14:23 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel, stephen, Yordan Karadzhov (VMware)
The example implements a small GUI that executes shell commands.
Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
kernel-shark/examples/CMakeLists.txt | 4 +++
kernel-shark/examples/cmd_split.cpp | 52 ++++++++++++++++++++++++++++
2 files changed, 56 insertions(+)
create mode 100644 kernel-shark/examples/cmd_split.cpp
diff --git a/kernel-shark/examples/CMakeLists.txt b/kernel-shark/examples/CMakeLists.txt
index e16216e..35e6b1e 100644
--- a/kernel-shark/examples/CMakeLists.txt
+++ b/kernel-shark/examples/CMakeLists.txt
@@ -23,3 +23,7 @@ target_link_libraries(dplot kshark-plot)
message(STATUS "widgetdemo")
add_executable(widgetdemo widgetdemo.cpp)
target_link_libraries(widgetdemo kshark-gui)
+
+message(STATUS "cmd_split")
+add_executable(cmd_split cmd_split.cpp)
+target_link_libraries(cmd_split kshark-gui)
diff --git a/kernel-shark/examples/cmd_split.cpp b/kernel-shark/examples/cmd_split.cpp
new file mode 100644
index 0000000..ac68844
--- /dev/null
+++ b/kernel-shark/examples/cmd_split.cpp
@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Copyright (C) 2017 VMware Inc, Yordan Karadzhov (VMware) <y.karadz@gmail.com>
+ */
+
+// C++
+#include<iostream>
+using namespace std;
+
+// Qt
+#include <QtWidgets>
+
+// KernelShark
+#include "KsUtils.hpp"
+
+int main(int argc, char **argv)
+{
+ QString text = "echo \"I want \\\" here\" \\\n \"and \\\' here\"";
+ QApplication a(argc, argv);
+ QStringList argList;
+ bool ok = true;
+ QProcess proc;
+
+ while (ok) {
+ text = QInputDialog::getMultiLineText(nullptr,
+ "Shell quoting test",
+ "Shell input:",
+ text,
+ &ok);
+
+ if (ok) {
+ argList = KsUtils::splitArguments(text);
+ qInfo() << argList;
+
+ proc.setProgram(argList.takeFirst());
+ proc.setArguments(argList);
+
+ proc.start();
+ proc.waitForFinished();
+
+ cout << proc.errorString().toStdString()
+ << endl
+ << proc.readAllStandardError().toStdString()
+ << endl
+ << proc.readAllStandardOutput().toStdString()
+ << endl;
+ }
+ }
+
+ return 0;
+}
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 3/3] kernel-shark: Add quotation marks parsing example/test
2019-09-18 14:23 ` [PATCH v2 3/3] kernel-shark: Add quotation marks parsing example/test Yordan Karadzhov (VMware)
@ 2019-09-19 23:12 ` Steven Rostedt
2019-09-20 9:50 ` Yordan Karadzhov (VMware)
0 siblings, 1 reply; 8+ messages in thread
From: Steven Rostedt @ 2019-09-19 23:12 UTC (permalink / raw)
To: Yordan Karadzhov (VMware); +Cc: linux-trace-devel, stephen
On Wed, 18 Sep 2019 17:23:19 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> The example implements a small GUI that executes shell commands.
>
> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
The testing on this seems to do what I think it should do.
I did modify it with this change, to get rid of the "Unknown error"
message on success. Care to send a v3?
-- Steve
diff --git a/kernel-shark/examples/cmd_split.cpp b/kernel-shark/examples/cmd_split.cpp
index ac688442..b8cc1b59 100644
--- a/kernel-shark/examples/cmd_split.cpp
+++ b/kernel-shark/examples/cmd_split.cpp
@@ -39,9 +39,10 @@ int main(int argc, char **argv)
proc.start();
proc.waitForFinished();
- cout << proc.errorString().toStdString()
- << endl
- << proc.readAllStandardError().toStdString()
+ if (proc.exitCode())
+ cout << proc.errorString().toStdString() << endl;
+
+ cout << proc.readAllStandardError().toStdString()
<< endl
<< proc.readAllStandardOutput().toStdString()
<< endl;
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 3/3] kernel-shark: Add quotation marks parsing example/test
2019-09-19 23:12 ` Steven Rostedt
@ 2019-09-20 9:50 ` Yordan Karadzhov (VMware)
0 siblings, 0 replies; 8+ messages in thread
From: Yordan Karadzhov (VMware) @ 2019-09-20 9:50 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linux-trace-devel, stephen
On 20.09.19 г. 2:12 ч., Steven Rostedt wrote:
> On Wed, 18 Sep 2019 17:23:19 +0300
> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
>
>> The example implements a small GUI that executes shell commands.
>>
>> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
>
> The testing on this seems to do what I think it should do.
>
> I did modify it with this change, to get rid of the "Unknown error"
> message on success. Care to send a v3?
I don't care, just get it upstream.
Thanks!
Yordan
>
> -- Steve
>
> diff --git a/kernel-shark/examples/cmd_split.cpp b/kernel-shark/examples/cmd_split.cpp
> index ac688442..b8cc1b59 100644
> --- a/kernel-shark/examples/cmd_split.cpp
> +++ b/kernel-shark/examples/cmd_split.cpp
> @@ -39,9 +39,10 @@ int main(int argc, char **argv)
> proc.start();
> proc.waitForFinished();
>
> - cout << proc.errorString().toStdString()
> - << endl
> - << proc.readAllStandardError().toStdString()
> + if (proc.exitCode())
> + cout << proc.errorString().toStdString() << endl;
> +
> + cout << proc.readAllStandardError().toStdString()
> << endl
> << proc.readAllStandardOutput().toStdString()
> << endl;
>
^ permalink raw reply [flat|nested] 8+ messages in thread