* [PATCH 2/3] kernel-shark: Handle properly the negative error codes when loading data
2019-05-23 15:09 [PATCH 0/3] Fixes toward KS 1.0 Yordan Karadzhov
2019-05-23 15:09 ` [PATCH 1/3] kernel-shark: Add Doxygen documentation for KsGLWidget::update() Yordan Karadzhov
@ 2019-05-23 15:09 ` Yordan Karadzhov
2019-05-23 15:09 ` [PATCH 3/3] kernel-shark: Correct memory management when data loading fails Yordan Karadzhov
2 siblings, 0 replies; 4+ messages in thread
From: Yordan Karadzhov @ 2019-05-23 15:09 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel, y.karadz, 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
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 3/3] kernel-shark: Correct memory management when data loading fails
2019-05-23 15:09 [PATCH 0/3] Fixes toward KS 1.0 Yordan Karadzhov
2019-05-23 15:09 ` [PATCH 1/3] kernel-shark: Add Doxygen documentation for KsGLWidget::update() Yordan Karadzhov
2019-05-23 15:09 ` [PATCH 2/3] kernel-shark: Handle properly the negative error codes when loading data Yordan Karadzhov
@ 2019-05-23 15:09 ` Yordan Karadzhov
2 siblings, 0 replies; 4+ messages in thread
From: Yordan Karadzhov @ 2019-05-23 15:09 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel, y.karadz, 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
^ permalink raw reply related [flat|nested] 4+ messages in thread