[PATCH 0/3] Fixes toward KS 1.0

[PATCH 0/3] Fixes toward KS 1.0

[Yordan Karadzhov]

Yordan Karadzhov (3):
  kernel-shark: Add Doxygen documentation for KsGLWidget::update()
  kernel-shark: Handle properly the negative error codes when loading
    data
  kernel-shark: Correct memory management when data loading fails

 kernel-shark/examples/datafilter.c |  6 +++++-
 kernel-shark/examples/datahisto.c  |  6 +++++-
 kernel-shark/examples/dataload.c   |  6 +++++-
 kernel-shark/src/KsGLWidget.hpp    |  1 +
 kernel-shark/src/KsMainWindow.cpp  |  9 ++++-----
 kernel-shark/src/KsUtils.cpp       |  6 +++---
 kernel-shark/src/KsUtils.hpp       |  4 ++--
 kernel-shark/src/libkshark.c       | 30 ++++++++++++++----------------
 8 files changed, 39 insertions(+), 29 deletions(-)

-- 
2.20.1

[PATCH 1/3] kernel-shark: Add Doxygen documentation for KsGLWidget::update()

[Yordan Karadzhov]

No functional changes in this patch.

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

diff --git a/kernel-shark/src/KsGLWidget.hpp b/kernel-shark/src/KsGLWidget.hpp
index 82831cf..e141b0a 100644
--- a/kernel-shark/src/KsGLWidget.hpp
+++ b/kernel-shark/src/KsGLWidget.hpp
@@ -41,6 +41,7 @@ public:
 
 	void reset();
 
+	/** Reprocess all graphs. */
 	void update() {resizeGL(width(), height());}
 
 	void mousePressEvent(QMouseEvent *event);
-- 
2.20.1

[PATCH 2/3] kernel-shark: Handle properly the negative error codes when loading data

[Yordan Karadzhov]

get_records() can return negative error codes. This means that we alway
have to use signed integer types for the "data size" variables / fields.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/examples/datafilter.c |  6 +++++-
 kernel-shark/examples/datahisto.c  |  6 +++++-
 kernel-shark/examples/dataload.c   |  6 +++++-
 kernel-shark/src/KsMainWindow.cpp  |  9 ++++-----
 kernel-shark/src/KsUtils.cpp       |  6 +++---
 kernel-shark/src/KsUtils.hpp       |  4 ++--
 kernel-shark/src/libkshark.c       | 13 +++++++------
 7 files changed, 31 insertions(+), 19 deletions(-)

diff --git a/kernel-shark/examples/datafilter.c b/kernel-shark/examples/datafilter.c
index 688e581..bebc181 100644
--- a/kernel-shark/examples/datafilter.c
+++ b/kernel-shark/examples/datafilter.c
@@ -15,7 +15,7 @@ const char *default_file = "trace.dat";
 
 int main(int argc, char **argv)
 {
-	size_t i, n_rows, n_tasks, n_evts, count;
+	ssize_t i, n_rows, n_tasks, n_evts, count;
 	struct kshark_context *kshark_ctx;
 	struct kshark_entry **data = NULL;
 	struct tep_event_filter *adv_filter;
@@ -42,6 +42,10 @@ int main(int argc, char **argv)
 
 	/* Load the content of the file into an array of entries. */
 	n_rows = kshark_load_data_entries(kshark_ctx, &data);
+	if (n_rows < 1) {
+		kshark_free(kshark_ctx);
+		return 1;
+	}
 
 	/* Filter the trace data coming from trace-cmd. */
 	n_tasks = kshark_get_task_pids(kshark_ctx, &pids);
diff --git a/kernel-shark/examples/datahisto.c b/kernel-shark/examples/datahisto.c
index 99ac495..02c6285 100644
--- a/kernel-shark/examples/datahisto.c
+++ b/kernel-shark/examples/datahisto.c
@@ -80,7 +80,7 @@ int main(int argc, char **argv)
 	struct kshark_context *kshark_ctx;
 	struct kshark_entry **data = NULL;
 	struct kshark_trace_histo histo;
-	size_t i, n_rows, n_tasks;
+	ssize_t i, n_rows, n_tasks;
 	bool status;
 	int *pids;
 
@@ -102,6 +102,10 @@ int main(int argc, char **argv)
 
 	/* Load the content of the file into an array of entries. */
 	n_rows = kshark_load_data_entries(kshark_ctx, &data);
+	if (n_rows < 1) {
+		kshark_free(kshark_ctx);
+		return 1;
+	}
 
 	/* Get a list of all tasks. */
 	n_tasks = kshark_get_task_pids(kshark_ctx, &pids);
diff --git a/kernel-shark/examples/dataload.c b/kernel-shark/examples/dataload.c
index 0cbc0f6..15c5de0 100644
--- a/kernel-shark/examples/dataload.c
+++ b/kernel-shark/examples/dataload.c
@@ -17,7 +17,7 @@ int main(int argc, char **argv)
 {
 	struct kshark_context *kshark_ctx;
 	struct kshark_entry **data = NULL;
-	size_t r, n_rows, n_tasks;
+	ssize_t r, n_rows, n_tasks;
 	char *entry_str;
 	bool status;
 	int *pids;
@@ -40,6 +40,10 @@ int main(int argc, char **argv)
 
 	/* Load the content of the file into an array of entries. */
 	n_rows = kshark_load_data_entries(kshark_ctx, &data);
+	if (n_rows < 1) {
+		kshark_free(kshark_ctx);
+		return 1;
+	}
 
 	/* Print to the screen the list of all tasks. */
 	n_tasks = kshark_get_task_pids(kshark_ctx, &pids);
diff --git a/kernel-shark/src/KsMainWindow.cpp b/kernel-shark/src/KsMainWindow.cpp
index c7c7f6b..1e03632 100644
--- a/kernel-shark/src/KsMainWindow.cpp
+++ b/kernel-shark/src/KsMainWindow.cpp
@@ -515,7 +515,7 @@ void KsMainWindow::_importFilter()
 	kshark_config_doc *conf;
 	QString fileName;
 
-	if (!kshark_instance(&kshark_ctx))
+	if (!kshark_instance(&kshark_ctx) || _data.size() < 1)
 		return;
 
 	fileName = KsUtils::getFile(this, "Import Filter",
@@ -995,11 +995,10 @@ void KsMainWindow::loadDataFile(const QString& fileName)
 
 	tload.join();
 
-	if (!_data.size()) {
-		QString text("File ");
+	if (_data.size() < 1) {
+		QString text("No data was loaded from file ");
 
-		text.append(fileName);
-		text.append(" contains no data.");
+		text.append(fileName + ".");
 		_error(text, "loadDataErr2", true, true);
 
 		return;
diff --git a/kernel-shark/src/KsUtils.cpp b/kernel-shark/src/KsUtils.cpp
index dcedd7b..58ce8c1 100644
--- a/kernel-shark/src/KsUtils.cpp
+++ b/kernel-shark/src/KsUtils.cpp
@@ -306,14 +306,14 @@ void KsDataStore::loadDataFile(const QString &file)
 
 void KsDataStore::_freeData()
 {
-	if (_dataSize) {
-		for (size_t r = 0; r < _dataSize; ++r)
+	if (_dataSize > 0) {
+		for (ssize_t r = 0; r < _dataSize; ++r)
 			free(_rows[r]);
 
 		free(_rows);
-		_rows = nullptr;
 	}
 
+	_rows = nullptr;
 	_dataSize = 0;
 }
 
diff --git a/kernel-shark/src/KsUtils.hpp b/kernel-shark/src/KsUtils.hpp
index b4663da..7362c14 100644
--- a/kernel-shark/src/KsUtils.hpp
+++ b/kernel-shark/src/KsUtils.hpp
@@ -161,7 +161,7 @@ public:
 	struct kshark_entry **rows() const {return _rows;}
 
 	/** Get the size of the data array. */
-	size_t size() const {return _dataSize;}
+	ssize_t size() const {return _dataSize;}
 
 	void reload();
 
@@ -198,7 +198,7 @@ private:
 	struct kshark_entry	**_rows;
 
 	/** The size of the data array. */
-	size_t			_dataSize;
+	ssize_t			_dataSize;
 
 	void _freeData();
 	void _unregisterCPUCollections();
diff --git a/kernel-shark/src/libkshark.c b/kernel-shark/src/libkshark.c
index 654aaa9..b0018f8 100644
--- a/kernel-shark/src/libkshark.c
+++ b/kernel-shark/src/libkshark.c
@@ -680,8 +680,8 @@ static void free_rec_list(struct rec_list **rec_list, int n_cpus,
 	free(rec_list);
 }
 
-static size_t get_records(struct kshark_context *kshark_ctx,
-			  struct rec_list ***rec_list, enum rec_type type)
+static ssize_t get_records(struct kshark_context *kshark_ctx,
+			   struct rec_list ***rec_list, enum rec_type type)
 {
 	struct kshark_event_handler *evt_handler;
 	struct tep_event_filter *adv_filter;
@@ -851,12 +851,12 @@ static int pick_next_cpu(struct rec_list **rec_list, int n_cpus,
  *	    negative error code on failure.
  */
 ssize_t kshark_load_data_entries(struct kshark_context *kshark_ctx,
-				struct kshark_entry ***data_rows)
+				 struct kshark_entry ***data_rows)
 {
 	struct kshark_entry **rows;
 	struct rec_list **rec_list;
 	enum rec_type type = REC_ENTRY;
-	size_t count, total = 0;
+	ssize_t count, total = 0;
 	int n_cpus;
 
 	if (*data_rows)
@@ -895,6 +895,7 @@ ssize_t kshark_load_data_entries(struct kshark_context *kshark_ctx,
 		free(rows[count]);
 	}
 	free(rows);
+
  fail:
 	fprintf(stderr, "Failed to allocate memory during data loading.\n");
 	return -ENOMEM;
@@ -913,14 +914,14 @@ ssize_t kshark_load_data_entries(struct kshark_context *kshark_ctx,
  *	    negative error code on failure.
  */
 ssize_t kshark_load_data_records(struct kshark_context *kshark_ctx,
-				struct tep_record ***data_rows)
+				 struct tep_record ***data_rows)
 {
 	struct tep_record **rows;
 	struct tep_record *rec;
 	struct rec_list **rec_list;
 	struct rec_list *temp_rec;
 	enum rec_type type = REC_RECORD;
-	size_t count, total = 0;
+	ssize_t count, total = 0;
 	int n_cpus;
 
 	total = get_records(kshark_ctx, &rec_list, REC_RECORD);
-- 
2.20.1

[PATCH 3/3] kernel-shark: Correct memory management when data loading fails

[Yordan Karadzhov]

The erroneous case, when the memory allocation failed during data loading,
hasn't been properly handled.

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

diff --git a/kernel-shark/src/libkshark.c b/kernel-shark/src/libkshark.c
index b0018f8..175279c 100644
--- a/kernel-shark/src/libkshark.c
+++ b/kernel-shark/src/libkshark.c
@@ -889,12 +889,6 @@ ssize_t kshark_load_data_entries(struct kshark_context *kshark_ctx,
 
  fail_free:
 	free_rec_list(rec_list, n_cpus, type);
-	for (count = 0; count < total; count++) {
-		if (!rows[count])
-			break;
-		free(rows[count]);
-	}
-	free(rows);
 
  fail:
 	fprintf(stderr, "Failed to allocate memory during data loading.\n");
@@ -924,15 +918,15 @@ ssize_t kshark_load_data_records(struct kshark_context *kshark_ctx,
 	ssize_t count, total = 0;
 	int n_cpus;
 
-	total = get_records(kshark_ctx, &rec_list, REC_RECORD);
+	total = get_records(kshark_ctx, &rec_list, type);
 	if (total < 0)
 		goto fail;
 
+	n_cpus = tracecmd_cpus(kshark_ctx->handle);
+
 	rows = calloc(total, sizeof(struct tep_record *));
 	if (!rows)
-		goto fail;
-
-	n_cpus = tracecmd_cpus(kshark_ctx->handle);
+		goto fail_free;
 
 	for (count = 0; count < total; count++) {
 		int next_cpu;
@@ -955,6 +949,9 @@ ssize_t kshark_load_data_records(struct kshark_context *kshark_ctx,
 	*data_rows = rows;
 	return total;
 
+ fail_free:
+	free_rec_list(rec_list, n_cpus, type);
+
  fail:
 	fprintf(stderr, "Failed to allocate memory during data loading.\n");
 	return -ENOMEM;
-- 
2.20.1