[PATCH 00/12] Various modifications and fixes toward KS 1.0

[PATCH 00/12] Various modifications and fixes toward KS 1.0

[Yordan Karadzhov]

Most of the modifications are minor except patch [PATCH 08/12] which
fixes a major memory leak.

Yordan Karadzhov (12):
  kernel-shark: Fix a spelling typo in KsMainWindow class
  kernel-shark: Fix a typo in an error message from libkshark-configio
  kernel-shark: Make KsSession::importFromFile return status flag
  kernel-shark: Error message if the opening of the session file fails
  kernel-shark: Disables automatic quotation for errors in KsMainWindow
  kernel-shark: Load Last Session from command line
  kernel-shark: Add destructor for KsGLWidget
  kernel-shark: Fix memory leak in KsPlotget::getTaskColorTable()
  kernel-shark: Avoid the unnecessary calls of KsGLWidget::loadColors()
  kernel-shark: Avoid 0/0 spliter ratio in KsSession
  kernel-shark: In model, handle the case when all bins are empty
  kernel-shark: In collections, handle the case when the data is small

 kernel-shark/src/KsGLWidget.cpp         |  8 ++++++--
 kernel-shark/src/KsGLWidget.hpp         |  2 ++
 kernel-shark/src/KsMainWindow.cpp       | 25 +++++++++++++++++--------
 kernel-shark/src/KsMainWindow.hpp       |  4 ++--
 kernel-shark/src/KsPlotTools.cpp        |  2 ++
 kernel-shark/src/KsSession.cpp          | 20 +++++++++++++++-----
 kernel-shark/src/KsSession.hpp          |  2 +-
 kernel-shark/src/kernelshark.cpp        |  9 ++++++++-
 kernel-shark/src/libkshark-collection.c | 20 ++++++++++++--------
 kernel-shark/src/libkshark-configio.c   |  2 +-
 kernel-shark/src/libkshark-model.c      |  9 +++++----
 11 files changed, 71 insertions(+), 32 deletions(-)

-- 
2.19.1

[PATCH 01/12] kernel-shark: Fix a spelling typo in KsMainWindow class

[Yordan Karadzhov]

_restorSession*  -> _restoreSession*
                           ^

Suggested-by: Slavomir Kaslev <kaslevs@vmware.com>
Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/src/KsMainWindow.cpp | 12 ++++++------
 kernel-shark/src/KsMainWindow.hpp |  4 ++--
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/kernel-shark/src/KsMainWindow.cpp b/kernel-shark/src/KsMainWindow.cpp
index a92944e..91e0c9f 100644
--- a/kernel-shark/src/KsMainWindow.cpp
+++ b/kernel-shark/src/KsMainWindow.cpp
@@ -43,7 +43,7 @@ KsMainWindow::KsMainWindow(QWidget *parent)
   _capture(this),
   _captureLocalServer(this),
   _openAction("Open", this),
-  _restorSessionAction("Restore Last Session", this),
+  _restoreSessionAction("Restore Last Session", this),
   _importSessionAction("Import Session", this),
   _exportSessionAction("Export Sassion", this),
   _quitAction("Quit", this),
@@ -167,9 +167,9 @@ void KsMainWindow::_createActions()
 	connect(&_openAction,	&QAction::triggered,
 		this,		&KsMainWindow::_open);
 
-	_restorSessionAction.setIcon(QIcon::fromTheme("document-open-recent"));
-	connect(&_restorSessionAction,	&QAction::triggered,
-		this,			&KsMainWindow::_restorSession);
+	_restoreSessionAction.setIcon(QIcon::fromTheme("document-open-recent"));
+	connect(&_restoreSessionAction,	&QAction::triggered,
+		this,			&KsMainWindow::_restoreSession);
 
 	_importSessionAction.setIcon(QIcon::fromTheme("document-send"));
 	_importSessionAction.setStatusTip("Load a session");
@@ -291,7 +291,7 @@ void KsMainWindow::_createMenus()
 
 	sessions = file->addMenu("Sessions");
 	sessions->setIcon(QIcon::fromTheme("document-properties"));
-	sessions->addAction(&_restorSessionAction);
+	sessions->addAction(&_restoreSessionAction);
 	sessions->addAction(&_importSessionAction);
 	sessions->addAction(&_exportSessionAction);
 	file->addAction(&_quitAction);
@@ -377,7 +377,7 @@ QString KsMainWindow::lastSessionFile()
 	return QString(file) +  "/lastsession.json";
 }
 
-void KsMainWindow::_restorSession()
+void KsMainWindow::_restoreSession()
 {
 	loadSession(lastSessionFile());
 	_graph.updateGeom();
diff --git a/kernel-shark/src/KsMainWindow.hpp b/kernel-shark/src/KsMainWindow.hpp
index c479520..1ecf2d8 100644
--- a/kernel-shark/src/KsMainWindow.hpp
+++ b/kernel-shark/src/KsMainWindow.hpp
@@ -99,7 +99,7 @@ private:
 	// File menu.
 	QAction		_openAction;
 
-	QAction		_restorSessionAction;
+	QAction		_restoreSessionAction;
 
 	QAction		_importSessionAction;
 
@@ -155,7 +155,7 @@ private:
 
 	void _open();
 
-	void _restorSession();
+	void _restoreSession();
 
 	void _importSession();
 
-- 
2.19.1

[PATCH 02/12] kernel-shark: Fix a typo in an error message from libkshark-configio

[Yordan Karadzhov]

The error message, printed whene kshark_open_json_file() fails, puts
the period on a new line.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/src/libkshark-configio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel-shark/src/libkshark-configio.c b/kernel-shark/src/libkshark-configio.c
index ab88864..2bd5600 100644
--- a/kernel-shark/src/libkshark-configio.c
+++ b/kernel-shark/src/libkshark-configio.c
@@ -1624,7 +1624,7 @@ static struct json_object *kshark_open_json_file(const char *file_name,
 
  fail:
 	/* The document has a wrong type. */
-	fprintf(stderr, "Failed to open Json file %s\n.", file_name);
+	fprintf(stderr, "Failed to open Json file %s.\n", file_name);
 	fprintf(stderr, "The document has a wrong type.\n");
 
 	json_object_put(jobj);
-- 
2.19.1

[PATCH 03/12] kernel-shark: Make KsSession::importFromFile return status flag

[Yordan Karadzhov]

The function has a better handling of the case when the session
description file cannot be loaded. It returns true on success,
otherwise false. We need this return flag in order to provide
adequate error message in the case when this operation fails.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/src/KsSession.cpp | 14 ++++++++++----
 kernel-shark/src/KsSession.hpp |  2 +-
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/kernel-shark/src/KsSession.cpp b/kernel-shark/src/KsSession.cpp
index 2242a12..b151818 100644
--- a/kernel-shark/src/KsSession.cpp
+++ b/kernel-shark/src/KsSession.cpp
@@ -28,13 +28,19 @@ KsSession::~KsSession()
 }
 
 /** Import a user session from a Json file. */
-void KsSession::importFromFile(QString jfileName)
+bool KsSession::importFromFile(QString jfileName)
 {
-	if (_config)
+	kshark_config_doc *configTmp =
+		kshark_open_config_file(jfileName.toStdString().c_str(),
+					"kshark.config.session");
+
+	if (configTmp) {
 		kshark_free_config_doc(_config);
+		_config = configTmp;
+		return true;
+	}
 
-	_config = kshark_open_config_file(jfileName.toStdString().c_str(),
-					  "kshark.config.session");
+	return false;
 }
 
 /** Export the current user session from a Json file. */
diff --git a/kernel-shark/src/KsSession.hpp b/kernel-shark/src/KsSession.hpp
index f5ed5a1..b07c810 100644
--- a/kernel-shark/src/KsSession.hpp
+++ b/kernel-shark/src/KsSession.hpp
@@ -37,7 +37,7 @@ public:
 	/** Get the configuration document object. */
 	kshark_config_doc *getConfDocPtr() const {return _config;}
 
-	void importFromFile(QString jfileName);
+	bool importFromFile(QString jfileName);
 
 	void exportToFile(QString jfileName);
 
-- 
2.19.1

[PATCH 04/12] kernel-shark: Error message if the opening of the session file fails

[Yordan Karadzhov]

Here we add proper handling of the case when the loading of the
session description file fails.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/src/KsMainWindow.cpp | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/kernel-shark/src/KsMainWindow.cpp b/kernel-shark/src/KsMainWindow.cpp
index 91e0c9f..44221f6 100644
--- a/kernel-shark/src/KsMainWindow.cpp
+++ b/kernel-shark/src/KsMainWindow.cpp
@@ -1010,7 +1010,16 @@ void KsMainWindow::loadSession(const QString &fileName)
 		return;
 	}
 
-	_session.importFromFile(fileName);
+	if (!_session.importFromFile(fileName)) {
+		QString text("Unable to open session description file ");
+
+		text.append(fileName);
+		text.append(".\n");
+		_error(text, "loadSessErr1", true, true);
+
+		return;
+	}
+
 	_session.loadPlugins(kshark_ctx, &_plugins);
 
 	QString dataFile(_session.getDataFile(kshark_ctx));
-- 
2.19.1

Re: [PATCH 04/12] kernel-shark: Error message if the opening of the session file fails

[Slavomir Kaslev]

On Thu, Mar 14, 2019 at 5:11 PM Yordan Karadzhov <ykaradzhov@vmware.com> wrote:
>
> Here we add proper handling of the case when the loading of the
> session description file fails.
>
> Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
> ---
>  kernel-shark/src/KsMainWindow.cpp | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/kernel-shark/src/KsMainWindow.cpp b/kernel-shark/src/KsMainWindow.cpp
> index 91e0c9f..44221f6 100644
> --- a/kernel-shark/src/KsMainWindow.cpp
> +++ b/kernel-shark/src/KsMainWindow.cpp
> @@ -1010,7 +1010,16 @@ void KsMainWindow::loadSession(const QString &fileName)
>                 return;
>         }
>
> -       _session.importFromFile(fileName);
> +       if (!_session.importFromFile(fileName)) {
> +               QString text("Unable to open session description file ");
> +
> +               text.append(fileName);
> +               text.append(".\n");

Imo this is cleaner with string interpolation, e.g.

        QString text = QString("Unable to open session description
file %1.\n").arg(fileName);

-- Slavi

> +               _error(text, "loadSessErr1", true, true);
> +
> +               return;
> +       }
> +
>         _session.loadPlugins(kshark_ctx, &_plugins);
>
>         QString dataFile(_session.getDataFile(kshark_ctx));
> --
> 2.19.1
>

[PATCH 05/12] kernel-shark: Disables automatic quotation for errors in KsMainWindow

[Yordan Karadzhov]

When quoting in the output stream is disabled, the text of the
message is printed without quotations and without escaping of
non-printable characters.

example output with quoting enabled:
ERROR:  "Unable to open trace data file for session mysession.json\n"

and with quoting disabled:
ERROR:  Unable to open session description file mysession.json.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/src/KsMainWindow.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel-shark/src/KsMainWindow.cpp b/kernel-shark/src/KsMainWindow.cpp
index 44221f6..9bcf9e2 100644
--- a/kernel-shark/src/KsMainWindow.cpp
+++ b/kernel-shark/src/KsMainWindow.cpp
@@ -980,7 +980,7 @@ void KsMainWindow::_error(const QString &text, const QString &errCode,
 	if (unloadPlugins)
 		_plugins.unloadAll();
 
-	qCritical() << "ERROR: " << text;
+	qCritical().noquote() << "ERROR: " << text;
 	em->showMessage(text, errCode);
 	em->exec();
 }
-- 
2.19.1

[PATCH 06/12] kernel-shark: Load Last Session from command line

[Yordan Karadzhov]

./kernelshark -l  will load directly the Last Session

Signed-off-by : Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/src/kernelshark.cpp | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/kernel-shark/src/kernelshark.cpp b/kernel-shark/src/kernelshark.cpp
index 2ec91de..1ec6678 100644
--- a/kernel-shark/src/kernelshark.cpp
+++ b/kernel-shark/src/kernelshark.cpp
@@ -28,6 +28,7 @@ void usage(const char *prog)
 	printf("  -p	register plugin, use plugin name, absolute or relative path\n");
 	printf("  -u	unregister plugin, use plugin name or absolute path\n");
 	printf("  -s	import a session\n");
+	printf("  -l	import the last session\n");
 }
 
 int main(int argc, char **argv)
@@ -40,7 +41,7 @@ int main(int argc, char **argv)
 	int c;
 	bool fromSession = false;
 
-	while ((c = getopt(argc, argv, "hvi:p:u:s:")) != -1) {
+	while ((c = getopt(argc, argv, "hvi:p:u:s:l")) != -1) {
 		switch(c) {
 		case 'h':
 			usage(argv[0]);
@@ -65,6 +66,12 @@ int main(int argc, char **argv)
 		case 's':
 			ks.loadSession(QString(optarg));
 			fromSession = true;
+			break;
+
+		case 'l':
+			ks.loadSession(ks.lastSessionFile());
+			fromSession = true;
+			break;
 
 		default:
 			break;
-- 
2.19.1

Re: [PATCH 06/12] kernel-shark: Load Last Session from command line

[Steven Rostedt]

On Thu, 14 Mar 2019 17:10:06 +0200
Yordan Karadzhov <ykaradzhov@vmware.com> wrote:

> ./kernelshark -l  will load directly the Last Session
> 
> Signed-off-by : Yordan Karadzhov <ykaradzhov@vmware.com>
> ---
>  kernel-shark/src/kernelshark.cpp | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel-shark/src/kernelshark.cpp b/kernel-shark/src/kernelshark.cpp
> index 2ec91de..1ec6678 100644
> --- a/kernel-shark/src/kernelshark.cpp
> +++ b/kernel-shark/src/kernelshark.cpp
> @@ -28,6 +28,7 @@ void usage(const char *prog)
>  	printf("  -p	register plugin, use plugin name, absolute or relative path\n");
>  	printf("  -u	unregister plugin, use plugin name or absolute path\n");
>  	printf("  -s	import a session\n");
> +	printf("  -l	import the last session\n");
>  }
>  
>  int main(int argc, char **argv)
> @@ -40,7 +41,7 @@ int main(int argc, char **argv)
>  	int c;
>  	bool fromSession = false;
>  
> -	while ((c = getopt(argc, argv, "hvi:p:u:s:")) != -1) {
> +	while ((c = getopt(argc, argv, "hvi:p:u:s:l")) != -1) {
>  		switch(c) {
>  		case 'h':
>  			usage(argv[0]);
> @@ -65,6 +66,12 @@ int main(int argc, char **argv)
>  		case 's':
>  			ks.loadSession(QString(optarg));
>  			fromSession = true;
> +			break;
> +
> +		case 'l':
> +			ks.loadSession(ks.lastSessionFile());
> +			fromSession = true;
> +			break;

This produces:

[ 69%] Building CXX object src/CMakeFiles/kernelshark.dir/kernelshark.cpp.o
/work/git/trace-cmd.git/kernel-shark/src/kernelshark.cpp: In function ‘int main(int, char**)’:
/work/git/trace-cmd.git/kernel-shark/src/kernelshark.cpp:72:22: error: ‘class KsMainWindow’ has no member named ‘lastSessionFile’; did you mean ‘loadSession’?
    ks.loadSession(ks.lastSessionFile());
                      ^~~~~~~~~~~~~~~
                      loadSession


Am I missing something?

-- Steve

>  
>  		default:
>  			break;

Re: [PATCH 06/12] kernel-shark: Load Last Session from command line

[Yordan Karadzhov (VMware)]

On 15.03.19 г. 1:47 ч., Steven Rostedt wrote:
> On Thu, 14 Mar 2019 17:10:06 +0200
> Yordan Karadzhov <ykaradzhov@vmware.com> wrote:
> 
>> ./kernelshark -l  will load directly the Last Session
>>
>> Signed-off-by : Yordan Karadzhov <ykaradzhov@vmware.com>
>> ---
>>   kernel-shark/src/kernelshark.cpp | 9 ++++++++-
>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel-shark/src/kernelshark.cpp b/kernel-shark/src/kernelshark.cpp
>> index 2ec91de..1ec6678 100644
>> --- a/kernel-shark/src/kernelshark.cpp
>> +++ b/kernel-shark/src/kernelshark.cpp
>> @@ -28,6 +28,7 @@ void usage(const char *prog)
>>   	printf("  -p	register plugin, use plugin name, absolute or relative path\n");
>>   	printf("  -u	unregister plugin, use plugin name or absolute path\n");
>>   	printf("  -s	import a session\n");
>> +	printf("  -l	import the last session\n");
>>   }
>>   
>>   int main(int argc, char **argv)
>> @@ -40,7 +41,7 @@ int main(int argc, char **argv)
>>   	int c;
>>   	bool fromSession = false;
>>   
>> -	while ((c = getopt(argc, argv, "hvi:p:u:s:")) != -1) {
>> +	while ((c = getopt(argc, argv, "hvi:p:u:s:l")) != -1) {
>>   		switch(c) {
>>   		case 'h':
>>   			usage(argv[0]);
>> @@ -65,6 +66,12 @@ int main(int argc, char **argv)
>>   		case 's':
>>   			ks.loadSession(QString(optarg));
>>   			fromSession = true;
>> +			break;
>> +
>> +		case 'l':
>> +			ks.loadSession(ks.lastSessionFile());
>> +			fromSession = true;
>> +			break;
> 
> This produces:
> 
> [ 69%] Building CXX object src/CMakeFiles/kernelshark.dir/kernelshark.cpp.o
> /work/git/trace-cmd.git/kernel-shark/src/kernelshark.cpp: In function ‘int main(int, char**)’:
> /work/git/trace-cmd.git/kernel-shark/src/kernelshark.cpp:72:22: error: ‘class KsMainWindow’ has no member named ‘lastSessionFile’; did you mean ‘loadSession’?
>      ks.loadSession(ks.lastSessionFile());
>                        ^~~~~~~~~~~~~~~
>                        loadSession
> 
> 
> Am I missing something?
> 

Looks like you are missing the two previous patch sets:

[PATCH v2 0/3]  Tuning the KernelShark build system
and
[PATCH v2 0/4] Add dialog for loading user-defined plugins

Thanks!
Yordan



> -- Steve
> 
>>   
>>   		default:
>>   			break;
> 

Re: [PATCH 06/12] kernel-shark: Load Last Session from command line

[Steven Rostedt]

On Fri, 15 Mar 2019 08:13:10 +0200
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> > Am I missing something?
> >   
> 
> Looks like you are missing the two previous patch sets:
> 
> [PATCH v2 0/3]  Tuning the KernelShark build system
> and
> [PATCH v2 0/4] Add dialog for loading user-defined plugins

Please make sure to mention dependencies in the 0/X patch change log.
Each patch set should be stand alone unless explicitly stated otherwise.

As when you get a lot of change series, you may go backwards in
accepting them (which is what I do).

-- Steve

[PATCH 07/12] kernel-shark: Add destructor for KsGLWidget

[Yordan Karadzhov]

Not sure how I missed to define this destructor before. The good
thing is that the GUI has only one GL widget, and this widget stays
alive for the whole duration of the program.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/src/KsGLWidget.cpp | 6 ++++++
 kernel-shark/src/KsGLWidget.hpp | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/kernel-shark/src/KsGLWidget.cpp b/kernel-shark/src/KsGLWidget.cpp
index 7a050c2..a2fcb8a 100644
--- a/kernel-shark/src/KsGLWidget.cpp
+++ b/kernel-shark/src/KsGLWidget.cpp
@@ -40,6 +40,12 @@ KsGLWidget::KsGLWidget(QWidget *parent)
 	connect(&_model, SIGNAL(modelReset()), this, SLOT(update()));
 }
 
+KsGLWidget::~KsGLWidget()
+{
+	for (auto &g: _graphs)
+		delete g;
+}
+
 /** Reimplemented function used to set up all required OpenGL resources. */
 void KsGLWidget::initializeGL()
 {
diff --git a/kernel-shark/src/KsGLWidget.hpp b/kernel-shark/src/KsGLWidget.hpp
index 95f307b..3bcecf9 100644
--- a/kernel-shark/src/KsGLWidget.hpp
+++ b/kernel-shark/src/KsGLWidget.hpp
@@ -31,6 +31,8 @@ class KsGLWidget : public QOpenGLWidget
 public:
 	explicit KsGLWidget(QWidget *parent = NULL);
 
+	~KsGLWidget();
+
 	void initializeGL() override;
 
 	void resizeGL(int w, int h) override;
-- 
2.19.1

[PATCH 08/12] kernel-shark: Fix memory leak in KsPlotget::getTaskColorTable()

[Yordan Karadzhov]

The std::vector constructor used here makes a copy of the data, so
the memory used by the original array has to be freed.

This is a major leak because getTaskColorTable() gets called every
time when we redraw the plots.

BTW calling getTaskColorTable() every time when we redraw makes
no sense. This will be fixed in the following patch.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/src/KsPlotTools.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel-shark/src/KsPlotTools.cpp b/kernel-shark/src/KsPlotTools.cpp
index d07f414..2b16a51 100644
--- a/kernel-shark/src/KsPlotTools.cpp
+++ b/kernel-shark/src/KsPlotTools.cpp
@@ -122,6 +122,8 @@ ColorTable getTaskColorTable()
 	std::vector<int> temp_pids(pids, pids + nTasks);
 	std::sort(temp_pids.begin(), temp_pids.end());
 
+	free(pids);
+
 	if (temp_pids[i] == 0) {
 		/* The "Idle" process (pid = 0) will be plotted in black. */
 		colors[i++] = {};
-- 
2.19.1

[PATCH 09/12] kernel-shark: Avoid the unnecessary calls of KsGLWidget::loadColors()

[Yordan Karadzhov]

The tables of colors used by the OpenGL widget have to be updated
only when we load a new trace data file.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/src/KsGLWidget.cpp | 2 --
 1 file changed, 2 deletions(-)

diff --git a/kernel-shark/src/KsGLWidget.cpp b/kernel-shark/src/KsGLWidget.cpp
index a2fcb8a..789514a 100644
--- a/kernel-shark/src/KsGLWidget.cpp
+++ b/kernel-shark/src/KsGLWidget.cpp
@@ -86,8 +86,6 @@ void KsGLWidget::paintGL()
 {
 	glClear(GL_COLOR_BUFFER_BIT);
 
-	loadColors();
-
 	/* Draw the time axis. */
 	if(_data)
 		_drawAxisX();
-- 
2.19.1

[PATCH 10/12] kernel-shark: Avoid 0/0 spliter ratio in KsSession

[Yordan Karadzhov]

Sometimes when we save the session QSplitter::sizes() returns
0/0 ratio (no idea why). Later when we load the session the
0/0 ratio results in misbehavior of the splitter position.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/src/KsSession.cpp | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/kernel-shark/src/KsSession.cpp b/kernel-shark/src/KsSession.cpp
index b151818..9d86776 100644
--- a/kernel-shark/src/KsSession.cpp
+++ b/kernel-shark/src/KsSession.cpp
@@ -256,7 +256,7 @@ void KsSession::loadSplitterSize(QSplitter *splitter)
 {
 	kshark_config_doc *spl = kshark_config_alloc(KS_CONFIG_JSON);
 	json_object *jspl, *jgraphsize, *jviewsize;
-	int graphSize, viewSize;
+	int graphSize(1), viewSize(1);
 	QList<int> sizes;
 
 	if (!kshark_config_doc_get(_config, "Splitter", spl))
@@ -269,6 +269,10 @@ void KsSession::loadSplitterSize(QSplitter *splitter)
 
 		graphSize = json_object_get_int(jgraphsize);
 		viewSize = json_object_get_int(jviewsize);
+		if (graphSize == 0 && viewSize == 0) {
+			/* 0/0 spliter ratio is undefined. Make it 1/1. */
+			viewSize = graphSize = 1;
+		}
 	}
 
 	sizes << graphSize << viewSize;
-- 
2.19.1

[PATCH 11/12] kernel-shark: In model, handle the case when all bins are empty

[Yordan Karadzhov]

ksmodel_set_bin_counts() should not crash in the case when all
bins of the model, except the Upper Overflow bin, are empty.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/src/libkshark-model.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/kernel-shark/src/libkshark-model.c b/kernel-shark/src/libkshark-model.c
index af3440b..29676c7 100644
--- a/kernel-shark/src/libkshark-model.c
+++ b/kernel-shark/src/libkshark-model.c
@@ -290,7 +290,7 @@ static void ksmodel_set_next_bin_edge(struct kshark_trace_histo *histo,
 static void ksmodel_set_bin_counts(struct kshark_trace_histo *histo)
 {
 	int i = 0, prev_not_empty;
-	ssize_t count_tmp;
+	ssize_t count_tmp = 0;
 
 	histo->tot_count = 0;
 	memset(&histo->bin_count[0], 0,
@@ -303,7 +303,7 @@ static void ksmodel_set_bin_counts(struct kshark_trace_histo *histo)
 		prev_not_empty = LOB(histo);
 	} else {
 		/* Loop till the first non-empty bin. */
-		while (histo->map[i] < 0) {
+		while (histo->map[i] < 0 && i < histo->n_bins) {
 			++i;
 		}
 
@@ -316,7 +316,8 @@ static void ksmodel_set_bin_counts(struct kshark_trace_histo *histo)
 	 */
 	for (; i < histo->n_bins; ++i) {
 		if (histo->map[i] != KS_EMPTY_BIN) {
-			/* The current bin is not empty, take its data row and
+			/*
+			 * The current bin is not empty, take its data row and
 			 * subtract it from the data row of the previous not
 			 * empty bin, which will give us the number of data
 			 * rows in the "prev_not_empty" bin.
@@ -358,7 +359,7 @@ static void ksmodel_set_bin_counts(struct kshark_trace_histo *histo)
 	 * We will do a sanity check. The number of data rows in the last not
 	 * empty bin must be greater than zero.
 	 */
-	assert(count_tmp > 0);
+	assert(count_tmp >= 0);
 	histo->tot_count += histo->bin_count[prev_not_empty] = count_tmp;
 }
 
-- 
2.19.1

Re: [PATCH 11/12] kernel-shark: In model, handle the case when all bins are empty

[Slavomir Kaslev]

On Thu, Mar 14, 2019 at 5:11 PM Yordan Karadzhov <ykaradzhov@vmware.com> wrote:
>
> ksmodel_set_bin_counts() should not crash in the case when all
> bins of the model, except the Upper Overflow bin, are empty.
>
> Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
> ---
>  kernel-shark/src/libkshark-model.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/kernel-shark/src/libkshark-model.c b/kernel-shark/src/libkshark-model.c
> index af3440b..29676c7 100644
> --- a/kernel-shark/src/libkshark-model.c
> +++ b/kernel-shark/src/libkshark-model.c
> @@ -290,7 +290,7 @@ static void ksmodel_set_next_bin_edge(struct kshark_trace_histo *histo,
>  static void ksmodel_set_bin_counts(struct kshark_trace_histo *histo)
>  {
>         int i = 0, prev_not_empty;
> -       ssize_t count_tmp;
> +       ssize_t count_tmp = 0;
>
>         histo->tot_count = 0;
>         memset(&histo->bin_count[0], 0,
> @@ -303,7 +303,7 @@ static void ksmodel_set_bin_counts(struct kshark_trace_histo *histo)
>                 prev_not_empty = LOB(histo);
>         } else {
>                 /* Loop till the first non-empty bin. */
> -               while (histo->map[i] < 0) {
> +               while (histo->map[i] < 0 && i < histo->n_bins) {

I would suggest switching the order of those checks

    while (i < histo->n_bins && histo->map[i] < 0) {

It's safe as is since hist->map has histo->n_bins+2 entries but
checking bounds before dereferencing is more idiomatic.

-- Slavi

>                         ++i;
>                 }
>
> @@ -316,7 +316,8 @@ static void ksmodel_set_bin_counts(struct kshark_trace_histo *histo)
>          */
>         for (; i < histo->n_bins; ++i) {
>                 if (histo->map[i] != KS_EMPTY_BIN) {
> -                       /* The current bin is not empty, take its data row and
> +                       /*
> +                        * The current bin is not empty, take its data row and
>                          * subtract it from the data row of the previous not
>                          * empty bin, which will give us the number of data
>                          * rows in the "prev_not_empty" bin.
> @@ -358,7 +359,7 @@ static void ksmodel_set_bin_counts(struct kshark_trace_histo *histo)
>          * We will do a sanity check. The number of data rows in the last not
>          * empty bin must be greater than zero.
>          */
> -       assert(count_tmp > 0);
> +       assert(count_tmp >= 0);
>         histo->tot_count += histo->bin_count[prev_not_empty] = count_tmp;
>  }
>
> --
> 2.19.1
>


-- 
Slavomir Kaslev

[PATCH 12/12] kernel-shark: In collections, handle the case when the data is small

[Yordan Karadzhov]

In particular, handle the case when the size of the data is smaller
than the required margin.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/src/libkshark-collection.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/kernel-shark/src/libkshark-collection.c b/kernel-shark/src/libkshark-collection.c
index c70da1e..02a014e 100644
--- a/kernel-shark/src/libkshark-collection.c
+++ b/kernel-shark/src/libkshark-collection.c
@@ -71,7 +71,7 @@ static bool collection_add_entry(struct entry_list **list,
 static struct kshark_entry_collection *
 kshark_data_collection_alloc(struct kshark_context *kshark_ctx,
 			     struct kshark_entry **data,
-			     size_t first,
+			     ssize_t first,
 			     size_t n_rows,
 			     matching_condition_func cond,
 			     int val,
@@ -81,9 +81,19 @@ kshark_data_collection_alloc(struct kshark_context *kshark_ctx,
 	struct kshark_entry *last_vis_entry = NULL;
 	struct entry_list *col_list, *temp;
 	size_t resume_count = 0, break_count = 0;
-	size_t i, j, end, last_added = 0;
+	size_t i, j, last_added = 0;
+	ssize_t end;
 	bool good_data = false;
 
+	/* Create the collection. */
+	col_ptr = calloc(1, sizeof(*col_ptr));
+	if (!col_ptr)
+		goto fail;
+
+	end = first + n_rows - margin;
+	if (first >= end)
+		return col_ptr;
+
 	col_list = malloc(sizeof(*col_list));
 	if (!col_list)
 		goto fail;
@@ -106,7 +116,6 @@ kshark_data_collection_alloc(struct kshark_context *kshark_ctx,
 		temp->type = COLLECTION_IGNORE;
 	}
 
-	end = first + n_rows - margin;
 	for (i = first + margin; i < end; ++i) {
 		if (!cond(kshark_ctx, data[i], val)) {
 			/*
@@ -204,11 +213,6 @@ kshark_data_collection_alloc(struct kshark_context *kshark_ctx,
 	 */
 	assert(break_count == resume_count);
 
-	/* Create the collection. */
-	col_ptr = malloc(sizeof(*col_ptr));
-	if (!col_ptr)
-		goto fail;
-
 	col_ptr->next = NULL;
 
 	col_ptr->resume_points = calloc(resume_count,
-- 
2.19.1