[PATCH 00/34] Fix kernelshark issues introduced by the migration to Qt6

[PATCH 00/34] Fix kernelshark issues introduced by the migration to Qt6

[Benjamin ROBIN]

There were 3 majors issues:
 - A segfault when loading a trace file (patch 0001)
 - The trace table height was very small (patch 0032)
 - The trace table columns width were reducing when clicking
   Marker A or B (patch 0032)

Also fix most of the warnings reported by Clang-Tidy and Clazy, and by
gcc with -Wextra.


Benjamin ROBIN (34):
  kernelshark: Fix modelReset() signaling, rename update to updateGeom
  kernelshark: Add .gitignore
  kernelshark: Remove function param when not used, whenever possible
  kernelshark: Do not create a temporary container for looping over QMap
  kernelshark: Prevent potential detach of QMap container
  kernelshark: Fix used after free of QByteArray raw data
  kernelshark: Fix potential memory leak in KsGLWidget
  kernelshark: Use lambda parameter instead of captured local variable
  kernelshark: Keep overridden method protected instead of public
  kernelshark: Use sliced() or first() instead of mid/right/left()
  kernelshark: Prevent potential divide by zero in Shape::center()
  kernelshark: Fix potential access to uninitialized variable
  kernelshark: Remove unused locals variables
  kernelshark: Fix range-loop-reference Clazy warning
  kernelshark: Fix moving a temp object prevents copy elision warning
  kernelshark: Add receiver object to connect() call
  kernelshark: Return by reference the list of header in KsModels
  kernelshark: Fix detaching-temporary Clazy warning
  kernelshark: Fix qfileinfo-exists Clazy warning
  kernelshark: Fix potential memory leaks in libkshark-configio
  kernelshark: Fix potential access to uninitialized variable
  kernelshark: Fix potential double free of histo->map, histo->bin_count
  kernelshark: Fix 'const' type qualifier on return type has no effect
  kernelshark: Fix potential memory leaks in libkshark-tepdata
  kernelshark: Fix typo in comment of KsGLWidget::resizeGL()
  kernelshark: Remove unused KsDataWidget::wipPtr() and broken function
  kernelshark: In KsTimeOffsetDialog() constructor use parent param
  kernelshark: Fixed loop counter incremented suspiciously twice
  kernelshark: Fix tepdata_dump_entry() for event_id = KS_EVENT_OVERFLOW
  kernelshark: Use static_cast instead of C cast in KsMainWindow
  kernelshark: Fix comparison of integers of different signs warnings
  kernelshark: Fix KsTableView columns width, and KsTraceViewer size
  kernelshark: Allow to reduce a bit more the graph height
  kernelshark: Cleanup of KsDualMarker methods

 .gitignore                     | 15 ++++++
 examples/configio.c            |  3 +-
 examples/datafilter.c          | 15 +++---
 examples/datahisto.c           |  2 +-
 src/KsAdvFilteringDialog.cpp   | 24 ++++------
 src/KsAdvFilteringDialog.hpp   |  2 +-
 src/KsDualMarker.hpp           | 10 +---
 src/KsGLWidget.cpp             | 48 +++++++++----------
 src/KsGLWidget.hpp             | 43 ++++++++---------
 src/KsMainWindow.cpp           |  8 ++--
 src/KsModels.hpp               | 11 +++--
 src/KsPlotTools.cpp            | 14 +++---
 src/KsPlotTools.hpp            |  2 +-
 src/KsSession.cpp              |  4 +-
 src/KsTraceGraph.cpp           |  7 ++-
 src/KsTraceViewer.cpp          | 71 ++++++++--------------------
 src/KsTraceViewer.hpp          | 11 +++--
 src/KsUtils.cpp                |  9 ++--
 src/KsUtils.hpp                |  4 +-
 src/KsWidgetsLib.cpp           |  2 +-
 src/KsWidgetsLib.hpp           | 15 ++----
 src/libkshark-collection.c     | 14 +++---
 src/libkshark-configio.c       | 84 +++++++++++++++++++---------------
 src/libkshark-hash.c           |  5 +-
 src/libkshark-model.c          | 19 ++++----
 src/libkshark-tepdata.c        | 31 ++++++++-----
 src/libkshark.c                | 17 +++----
 src/libkshark.h                | 20 ++++----
 src/plugins/KVMComboDialog.cpp |  7 +--
 src/plugins/sched_events.c     |  2 +-
 tests/test-input.c             |  4 +-
 tests/test-input_ctrl.c        |  4 +-
 32 files changed, 257 insertions(+), 270 deletions(-)
 create mode 100644 .gitignore

--
2.43.0

[PATCH 01/34] kernelshark: Fix modelReset() signaling, rename update to updateGeom

[Benjamin ROBIN]

Fix segfault introduced by the migration to Qt6.

There was a public update() function in KsGLWidget class which overrides
QWidget::update(). The QAbstractTableModel::modelReset signal was connected
to the QWidget::update slot using "old" connect syntax. This was working
since QWidget::update was declared as a slot, and registered in QWidget
meta information.

When migrating to Qt6, the new connect syntax was used, which accidentally
connect the KsGLWidget::update function instead of QWidget::update.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=218350
Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/KsGLWidget.cpp   | 4 ++--
 src/KsGLWidget.hpp   | 2 +-
 src/KsTraceGraph.cpp | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/KsGLWidget.cpp b/src/KsGLWidget.cpp
index 023d1c8..9e3dac3 100644
--- a/src/KsGLWidget.cpp
+++ b/src/KsGLWidget.cpp
@@ -53,7 +53,7 @@ KsGLWidget::KsGLWidget(QWidget *parent)
 	setMouseTracking(true);
 
 	connect(&_model,	&QAbstractTableModel::modelReset,
-		this,		&KsGLWidget::update);
+		this,		qOverload<>(&KsGLWidget::update));
 }
 
 void KsGLWidget::_freeGraphs()
@@ -89,7 +89,7 @@ void KsGLWidget::initializeGL()
 	ksplot_init_font(&_font, 15, TT_FONT_FILE);
 
 	_labelSize = _getMaxLabelSize() + FONT_WIDTH * 2;
-	update();
+	updateGeom();
 }
 
 /**
diff --git a/src/KsGLWidget.hpp b/src/KsGLWidget.hpp
index 03bd5eb..1c6253f 100644
--- a/src/KsGLWidget.hpp
+++ b/src/KsGLWidget.hpp
@@ -86,7 +86,7 @@ public:
 	void reset();
 
 	/** Reprocess all graphs. */
-	void update() {resizeGL(width(), height());}
+	void updateGeom() {resizeGL(width(), height());}
 
 	void mousePressEvent(QMouseEvent *event);
 
diff --git a/src/KsTraceGraph.cpp b/src/KsTraceGraph.cpp
index 65e5a79..f80477d 100644
--- a/src/KsTraceGraph.cpp
+++ b/src/KsTraceGraph.cpp
@@ -614,7 +614,7 @@ void KsTraceGraph::updateGeom()
 			       * widget is extended to maximum.
 			       */
 
-	_glWindow.update();
+	_glWindow.updateGeom();
 }
 
 /**
-- 
2.43.0

[PATCH 02/34] kernelshark: Add .gitignore

[Benjamin ROBIN]

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 .gitignore | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
 create mode 100644 .gitignore

diff --git a/.gitignore b/.gitignore
new file mode 100644
index 0000000..87a23cc
--- /dev/null
+++ b/.gitignore
@@ -0,0 +1,15 @@
+
+# Ignore everything in bin directory except kshark-su-record
+/bin/*
+!/bin/kshark-su-record
+
+# Ignore generated files
+CMakeLists.txt.user
+/libkshark.pc
+/org.freedesktop.kshark-record.policy
+/kernelshark.desktop
+/src/KsCmakeDef.hpp
+/lib/*
+/tests/*.so
+/tests/kshark-tests
+/tests/kshark-gui-tests
-- 
2.43.0

[PATCH 03/34] kernelshark: Remove function param when not used, whenever possible

[Benjamin ROBIN]

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/KsAdvFilteringDialog.cpp | 4 ++--
 src/KsAdvFilteringDialog.hpp | 2 +-
 src/KsMainWindow.cpp         | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/KsAdvFilteringDialog.cpp b/src/KsAdvFilteringDialog.cpp
index b934db6..4683c3d 100644
--- a/src/KsAdvFilteringDialog.cpp
+++ b/src/KsAdvFilteringDialog.cpp
@@ -80,7 +80,7 @@ KsAdvFilteringDialog::KsAdvFilteringDialog(QWidget *parent)
 	_getFilters(kshark_ctx);
 
 	if (_filters.count()) {
-		_makeFilterTable(kshark_ctx);
+		_makeFilterTable();
 		lamAddLine();
 	}
 
@@ -264,7 +264,7 @@ void KsAdvFilteringDialog::_getFilters(kshark_context *kshark_ctx)
 	}
 }
 
-void KsAdvFilteringDialog::_makeFilterTable(struct kshark_context *kshark_ctx)
+void KsAdvFilteringDialog::_makeFilterTable()
 {
 	QMapIterator<int, QString> f(_filters);
 	QTableWidgetItem *i1, *i2, *i3;
diff --git a/src/KsAdvFilteringDialog.hpp b/src/KsAdvFilteringDialog.hpp
index a23e8d9..30bbadc 100644
--- a/src/KsAdvFilteringDialog.hpp
+++ b/src/KsAdvFilteringDialog.hpp
@@ -80,7 +80,7 @@ private:
 
 	void _getFilters(kshark_context *kshark_ctx);
 
-	void _makeFilterTable(kshark_context *kshark_ctx);
+	void _makeFilterTable();
 
 	QStringList _getEventFields(int eventId);
 
diff --git a/src/KsMainWindow.cpp b/src/KsMainWindow.cpp
index ef15b20..68853d6 100644
--- a/src/KsMainWindow.cpp
+++ b/src/KsMainWindow.cpp
@@ -1434,12 +1434,12 @@ void KsMainWindow::loadSession(const QString &fileName)
 	_session.loadUserPlugins(kshark_ctx, &_plugins);
 	pb.setValue(20);
 
-	auto lamLoadJob = [&] (KsDataStore *d) {
+	auto lamLoadJob = [&] () {
 		_session.loadDataStreams(kshark_ctx, &_data);
 		loadDone = true;
 	};
 
-	std::thread job = std::thread(lamLoadJob, &_data);
+	std::thread job = std::thread(lamLoadJob);
 
 	for (int i = 0; i < 150; ++i) {
 		/*
-- 
2.43.0

[PATCH 04/34] kernelshark: Do not create a temporary container for looping over QMap

[Benjamin ROBIN]

Use const_iterator instead. Fix container-anti-pattern Clazy warning

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/KsAdvFilteringDialog.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/KsAdvFilteringDialog.cpp b/src/KsAdvFilteringDialog.cpp
index 4683c3d..247f912 100644
--- a/src/KsAdvFilteringDialog.cpp
+++ b/src/KsAdvFilteringDialog.cpp
@@ -276,8 +276,8 @@ void KsAdvFilteringDialog::_makeFilterTable()
 	headers << "Delete" << "Stream" << "Event" << " Id" << "Filter";
 	_table->init(headers, _filters.count());
 
-	for(auto f : _filters.keys()) {
-		QStringList thisFilter = _filters.value(f).split(":");
+	for (auto it = _filters.cbegin(), end = _filters.cend(); it != end; ++it) {
+		QStringList thisFilter = it.value().split(":");
 
 		i1 = new QTableWidgetItem(thisFilter[0]);
 		_table->setItem(count, 1, i1);
@@ -285,7 +285,7 @@ void KsAdvFilteringDialog::_makeFilterTable()
 		i1 = new QTableWidgetItem(thisFilter[1]);
 		_table->setItem(count, 2, i1);
 
-		i2 = new QTableWidgetItem(tr("%1").arg(f));
+		i2 = new QTableWidgetItem(tr("%1").arg(it.key()));
 		_table->setItem(count, 3, i2);
 
 		i3 = new QTableWidgetItem(thisFilter[2]);
-- 
2.43.0

Re: [PATCH 04/34] kernelshark: Do not create a temporary container for looping over QMap

[Yordan Karadzhov]

On 1/14/24 19:16, Benjamin ROBIN wrote:
> Use const_iterator instead. Fix container-anti-pattern Clazy warning
> 
> Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
> ---
>   src/KsAdvFilteringDialog.cpp | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/KsAdvFilteringDialog.cpp b/src/KsAdvFilteringDialog.cpp
> index 4683c3d..247f912 100644
> --- a/src/KsAdvFilteringDialog.cpp
> +++ b/src/KsAdvFilteringDialog.cpp
> @@ -276,8 +276,8 @@ void KsAdvFilteringDialog::_makeFilterTable()
>   	headers << "Delete" << "Stream" << "Event" << " Id" << "Filter";
>   	_table->init(headers, _filters.count());
>   
> -	for(auto f : _filters.keys()) {
> -		QStringList thisFilter = _filters.value(f).split(":");
> +	for (auto it = _filters.cbegin(), end = _filters.cend(); it != end; ++it) {

Do we need to use iterator here?
Perhaps you can do something like:

for (const auto &[key, val] : _filters) {

Thanks!
Y.

> +		QStringList thisFilter = it.value().split(":");
>   
>   		i1 = new QTableWidgetItem(thisFilter[0]);
>   		_table->setItem(count, 1, i1);
> @@ -285,7 +285,7 @@ void KsAdvFilteringDialog::_makeFilterTable()
>   		i1 = new QTableWidgetItem(thisFilter[1]);
>   		_table->setItem(count, 2, i1);
>   
> -		i2 = new QTableWidgetItem(tr("%1").arg(f));
> +		i2 = new QTableWidgetItem(tr("%1").arg(it.key()));
>   		_table->setItem(count, 3, i2);
>   
>   		i3 = new QTableWidgetItem(thisFilter[2]);

Re: [PATCH 04/34] kernelshark: Do not create a temporary container for looping over QMap

[Benjamin ROBIN]

On Sun, Jan 21, 2024 at 07:16:01PM +0200, Yordan Karadzhov wrote:
> 
> 
> On 1/14/24 19:16, Benjamin ROBIN wrote:
> > Use const_iterator instead. Fix container-anti-pattern Clazy warning
> > 
> > Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
> > ---
> >   src/KsAdvFilteringDialog.cpp | 6 +++---
> >   1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/KsAdvFilteringDialog.cpp b/src/KsAdvFilteringDialog.cpp
> > index 4683c3d..247f912 100644
> > --- a/src/KsAdvFilteringDialog.cpp
> > +++ b/src/KsAdvFilteringDialog.cpp
> > @@ -276,8 +276,8 @@ void KsAdvFilteringDialog::_makeFilterTable()
> >   	headers << "Delete" << "Stream" << "Event" << " Id" << "Filter";
> >   	_table->init(headers, _filters.count());
> > -	for(auto f : _filters.keys()) {
> > -		QStringList thisFilter = _filters.value(f).split(":");
> > +	for (auto it = _filters.cbegin(), end = _filters.cend(); it != end; ++it) {
> 
> Do we need to use iterator here?
> Perhaps you can do something like:
> 
> for (const auto &[key, val] : _filters) {

Unfortunately, you cannot do that, as far as I know (this not compile).
C++ range-based for loop is using iterator behind the scene. See [1].
And there is still the problem of range-loop-detach (see patch 0005).
Using const iterator is the most explicit and safe way to iterate over a
container in Qt, so why not using it?

[1] https://en.cppreference.com/w/cpp/language/range-for
 
> Thanks!
> Y.
> 
> > +		QStringList thisFilter = it.value().split(":");
> >   		i1 = new QTableWidgetItem(thisFilter[0]);
> >   		_table->setItem(count, 1, i1);
> > @@ -285,7 +285,7 @@ void KsAdvFilteringDialog::_makeFilterTable()
> >   		i1 = new QTableWidgetItem(thisFilter[1]);
> >   		_table->setItem(count, 2, i1);
> > -		i2 = new QTableWidgetItem(tr("%1").arg(f));
> > +		i2 = new QTableWidgetItem(tr("%1").arg(it.key()));
> >   		_table->setItem(count, 3, i2);
> >   		i3 = new QTableWidgetItem(thisFilter[2]);

Re: [PATCH 04/34] kernelshark: Do not create a temporary container for looping over QMap

[Yordan Karadzhov]

On 1/28/24 22:30, Benjamin ROBIN wrote:
> On Sun, Jan 21, 2024 at 07:16:01PM +0200, Yordan Karadzhov wrote:
>>
>>
>> On 1/14/24 19:16, Benjamin ROBIN wrote:
>>> Use const_iterator instead. Fix container-anti-pattern Clazy warning
>>>
>>> Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
>>> ---
>>>    src/KsAdvFilteringDialog.cpp | 6 +++---
>>>    1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/src/KsAdvFilteringDialog.cpp b/src/KsAdvFilteringDialog.cpp
>>> index 4683c3d..247f912 100644
>>> --- a/src/KsAdvFilteringDialog.cpp
>>> +++ b/src/KsAdvFilteringDialog.cpp
>>> @@ -276,8 +276,8 @@ void KsAdvFilteringDialog::_makeFilterTable()
>>>    	headers << "Delete" << "Stream" << "Event" << " Id" << "Filter";
>>>    	_table->init(headers, _filters.count());
>>> -	for(auto f : _filters.keys()) {
>>> -		QStringList thisFilter = _filters.value(f).split(":");
>>> +	for (auto it = _filters.cbegin(), end = _filters.cend(); it != end; ++it) {
>>
>> Do we need to use iterator here?
>> Perhaps you can do something like:
>>
>> for (const auto &[key, val] : _filters) {
> 
> Unfortunately, you cannot do that, as far as I know (this not compile).
> C++ range-based for loop is using iterator behind the scene. See [1].
> And there is still the problem of range-loop-detach (see patch 0005).
> Using const iterator is the most explicit and safe way to iterate over a
> container in Qt, so why not using it?

I see your point. I am applying this patch together with all v2 patches 
you sent.

Are you considering sending new version of the "[31/34] Fix comparison 
of integers of different signs warnings"?

Thanks,
Y.

> 
> [1] https://en.cppreference.com/w/cpp/language/range-for
>   
>> Thanks!
>> Y.
>>
>>> +		QStringList thisFilter = it.value().split(":");
>>>    		i1 = new QTableWidgetItem(thisFilter[0]);
>>>    		_table->setItem(count, 1, i1);
>>> @@ -285,7 +285,7 @@ void KsAdvFilteringDialog::_makeFilterTable()
>>>    		i1 = new QTableWidgetItem(thisFilter[1]);
>>>    		_table->setItem(count, 2, i1);
>>> -		i2 = new QTableWidgetItem(tr("%1").arg(f));
>>> +		i2 = new QTableWidgetItem(tr("%1").arg(it.key()));
>>>    		_table->setItem(count, 3, i2);
>>>    		i3 = new QTableWidgetItem(thisFilter[2]);

Re: [PATCH 04/34] kernelshark: Do not create a temporary container for looping over QMap

[Benjamin ROBIN]

On Sun, Feb 04, 2024 at 08:34:39PM +0200, Yordan Karadzhov wrote:
> 
> 
> On 1/28/24 22:30, Benjamin ROBIN wrote:
> > On Sun, Jan 21, 2024 at 07:16:01PM +0200, Yordan Karadzhov wrote:
> > > 
> > > 
> > > On 1/14/24 19:16, Benjamin ROBIN wrote:
> > > > Use const_iterator instead. Fix container-anti-pattern Clazy warning
> > > > 
> > > > Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
> > > > ---
> > > >    src/KsAdvFilteringDialog.cpp | 6 +++---
> > > >    1 file changed, 3 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/src/KsAdvFilteringDialog.cpp b/src/KsAdvFilteringDialog.cpp
> > > > index 4683c3d..247f912 100644
> > > > --- a/src/KsAdvFilteringDialog.cpp
> > > > +++ b/src/KsAdvFilteringDialog.cpp
> > > > @@ -276,8 +276,8 @@ void KsAdvFilteringDialog::_makeFilterTable()
> > > >    	headers << "Delete" << "Stream" << "Event" << " Id" << "Filter";
> > > >    	_table->init(headers, _filters.count());
> > > > -	for(auto f : _filters.keys()) {
> > > > -		QStringList thisFilter = _filters.value(f).split(":");
> > > > +	for (auto it = _filters.cbegin(), end = _filters.cend(); it != end; ++it) {
> > > 
> > > Do we need to use iterator here?
> > > Perhaps you can do something like:
> > > 
> > > for (const auto &[key, val] : _filters) {
> > 
> > Unfortunately, you cannot do that, as far as I know (this not compile).
> > C++ range-based for loop is using iterator behind the scene. See [1].
> > And there is still the problem of range-loop-detach (see patch 0005).
> > Using const iterator is the most explicit and safe way to iterate over a
> > container in Qt, so why not using it?
> 
> I see your point. I am applying this patch together with all v2 patches you
> sent.
> 
> Are you considering sending new version of the "[31/34] Fix comparison of
> integers of different signs warnings"?

I did not have time last weekend, nor this weekend to take a deep loop into it. 
Fixing types is not that easy:
 - I need a better understanding of the code base, otherwise I will break stuff.
 - I don't know if there are other projects that rely on these lib headers
   files. Is it possible to refactor every part of the project without breaking 
   something else?
 - My patch tried to fix at various places warnings using mainly cast, but there
   are so many other places where the types are wrong (or at least not the optimal
   one).
 - This task is not that small. It requires a bit more time.

So, in summary, this patch is currently on hold. I may work on it later this 
year... Sorry about that.

Thanks,
Benjamin

> > 
> > [1] https://en.cppreference.com/w/cpp/language/range-for
> > > Thanks!
> > > Y.
> > > 
> > > > +		QStringList thisFilter = it.value().split(":");
> > > >    		i1 = new QTableWidgetItem(thisFilter[0]);
> > > >    		_table->setItem(count, 1, i1);
> > > > @@ -285,7 +285,7 @@ void KsAdvFilteringDialog::_makeFilterTable()
> > > >    		i1 = new QTableWidgetItem(thisFilter[1]);
> > > >    		_table->setItem(count, 2, i1);
> > > > -		i2 = new QTableWidgetItem(tr("%1").arg(f));
> > > > +		i2 = new QTableWidgetItem(tr("%1").arg(it.key()));
> > > >    		_table->setItem(count, 3, i2);
> > > >    		i3 = new QTableWidgetItem(thisFilter[2]);

[PATCH 05/34] kernelshark: Prevent potential detach of QMap container

[Benjamin ROBIN]

Use const_iterator instead. Fix range-loop-detach Clazy warning

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/KsGLWidget.cpp             | 5 +++--
 src/plugins/KVMComboDialog.cpp | 6 ++++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/KsGLWidget.cpp b/src/KsGLWidget.cpp
index 9e3dac3..0a44e77 100644
--- a/src/KsGLWidget.cpp
+++ b/src/KsGLWidget.cpp
@@ -137,9 +137,10 @@ void KsGLWidget::paintGL()
 	/* Draw the time axis. */
 	_drawAxisX(size);
 
-	for (auto const &stream: _graphs)
-		for (auto const &g: stream)
+	for (auto it = _graphs.cbegin(), end = _graphs.cend(); it != end; ++it) {
+		for (auto const &g: it.value())
 			g->draw(size);
+	}
 
 	for (auto const &s: _shapes) {
 		if (!s)
diff --git a/src/plugins/KVMComboDialog.cpp b/src/plugins/KVMComboDialog.cpp
index 2b95a53..6be68d4 100644
--- a/src/plugins/KVMComboDialog.cpp
+++ b/src/plugins/KVMComboDialog.cpp
@@ -308,13 +308,15 @@ void KsComboPlotDialog::_applyPress()
 	int nPlots(0);
 
 	_plotMap[guestId] = _streamCombos(guestId);
-	for (auto const &stream: _plotMap)
-		for (auto const &combo: stream) {
+
+	for (auto it = _plotMap.cbegin(), end = _plotMap.cend(); it != end; ++it) {
+			for (auto const &combo: it.value()) {
 			allCombosVec.append(2);
 			combo[0] >> allCombosVec;
 			combo[1] >> allCombosVec;
 			++nPlots;
 		}
+	}
 
 	emit apply(nPlots, allCombosVec);
 }
-- 
2.43.0

Re: [PATCH 05/34] kernelshark: Prevent potential detach of QMap container

[Yordan Karadzhov]

On 1/14/24 19:16, Benjamin ROBIN wrote:
> Use const_iterator instead. Fix range-loop-detach Clazy warning
> 

Please provide a better explanation of the problem you are trying
to solve with this patch.

Thanks!
Y.

> Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
> ---
>   src/KsGLWidget.cpp             | 5 +++--
>   src/plugins/KVMComboDialog.cpp | 6 ++++--
>   2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/src/KsGLWidget.cpp b/src/KsGLWidget.cpp
> index 9e3dac3..0a44e77 100644
> --- a/src/KsGLWidget.cpp
> +++ b/src/KsGLWidget.cpp
> @@ -137,9 +137,10 @@ void KsGLWidget::paintGL()
>   	/* Draw the time axis. */
>   	_drawAxisX(size);
>   
> -	for (auto const &stream: _graphs)
> -		for (auto const &g: stream)
> +	for (auto it = _graphs.cbegin(), end = _graphs.cend(); it != end; ++it) {
> +		for (auto const &g: it.value())
>   			g->draw(size);
> +	}
>   
>   	for (auto const &s: _shapes) {
>   		if (!s)
> diff --git a/src/plugins/KVMComboDialog.cpp b/src/plugins/KVMComboDialog.cpp
> index 2b95a53..6be68d4 100644
> --- a/src/plugins/KVMComboDialog.cpp
> +++ b/src/plugins/KVMComboDialog.cpp
> @@ -308,13 +308,15 @@ void KsComboPlotDialog::_applyPress()
>   	int nPlots(0);
>   
>   	_plotMap[guestId] = _streamCombos(guestId);
> -	for (auto const &stream: _plotMap)
> -		for (auto const &combo: stream) {
> +
> +	for (auto it = _plotMap.cbegin(), end = _plotMap.cend(); it != end; ++it) {
> +			for (auto const &combo: it.value()) {
>   			allCombosVec.append(2);
>   			combo[0] >> allCombosVec;
>   			combo[1] >> allCombosVec;
>   			++nPlots;
>   		}
> +	}
>   
>   	emit apply(nPlots, allCombosVec);
>   }

[PATCH v2 05/34] kernelshark: Prevent potential detach of QMap container

[Benjamin ROBIN]

Use const_iterator instead. Fix range-loop-detach Clazy warning.
Indeed when using C++11 range-loops, the .begin() and .end() functions are
called, instead of .cbegin() and .cend(). This imply before looping over the
QMap, it may perform a deep-copy of it (if shared).

See also the explanation given by Qt documentation of qAsConst().
Another solution is to use "std::as_const" cast on the QMap object.

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/KsGLWidget.cpp             | 5 +++--
 src/plugins/KVMComboDialog.cpp | 6 ++++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/KsGLWidget.cpp b/src/KsGLWidget.cpp
index 9e3dac3..0a44e77 100644
--- a/src/KsGLWidget.cpp
+++ b/src/KsGLWidget.cpp
@@ -137,9 +137,10 @@ void KsGLWidget::paintGL()
 	/* Draw the time axis. */
 	_drawAxisX(size);
 
-	for (auto const &stream: _graphs)
-		for (auto const &g: stream)
+	for (auto it = _graphs.cbegin(), end = _graphs.cend(); it != end; ++it) {
+		for (auto const &g: it.value())
 			g->draw(size);
+	}
 
 	for (auto const &s: _shapes) {
 		if (!s)
diff --git a/src/plugins/KVMComboDialog.cpp b/src/plugins/KVMComboDialog.cpp
index 2b95a53..6be68d4 100644
--- a/src/plugins/KVMComboDialog.cpp
+++ b/src/plugins/KVMComboDialog.cpp
@@ -308,13 +308,15 @@ void KsComboPlotDialog::_applyPress()
 	int nPlots(0);
 
 	_plotMap[guestId] = _streamCombos(guestId);
-	for (auto const &stream: _plotMap)
-		for (auto const &combo: stream) {
+
+	for (auto it = _plotMap.cbegin(), end = _plotMap.cend(); it != end; ++it) {
+			for (auto const &combo: it.value()) {
 			allCombosVec.append(2);
 			combo[0] >> allCombosVec;
 			combo[1] >> allCombosVec;
 			++nPlots;
 		}
+	}
 
 	emit apply(nPlots, allCombosVec);
 }
-- 
2.43.0

[PATCH 06/34] kernelshark: Fix used after free of QByteArray raw data

[Benjamin ROBIN]

In KsAdvFilteringDialog::_applyPress(), QByteArray raw data, obtained
from _filterEdit, is accessed after being freed.
Also prevent any unnecessary copy.

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/KsAdvFilteringDialog.cpp | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/src/KsAdvFilteringDialog.cpp b/src/KsAdvFilteringDialog.cpp
index 247f912..c0d6d48 100644
--- a/src/KsAdvFilteringDialog.cpp
+++ b/src/KsAdvFilteringDialog.cpp
@@ -443,8 +443,6 @@ void KsAdvFilteringDialog::_applyPress()
 	QMapIterator<int, QString> f(_filters);
 	kshark_context *kshark_ctx(NULL);
 	kshark_data_stream *stream;
-	const char *text;
-	char *filter;
 	int i(0);
 
 	if (!kshark_instance(&kshark_ctx))
@@ -476,18 +474,12 @@ void KsAdvFilteringDialog::_applyPress()
 		emit dataReload();
 	};
 
-	text = _filterEdit.text().toLocal8Bit().data();
-	if (strlen(text) == 0) {
+	QByteArray filter = _filterEdit.text().toLocal8Bit();
+	if (filter.isEmpty()) {
 		job_done();
 		return;
 	}
 
-	filter = (char*) malloc(strlen(text) + 1);
-	strcpy(filter, text);
-
-	kshark_tep_add_filter_str(stream, filter);
-
-	free(filter);
-
+	kshark_tep_add_filter_str(stream, filter.constData());
 	job_done();
 }
-- 
2.43.0

[PATCH 07/34] kernelshark: Fix potential memory leak in KsGLWidget

[Benjamin ROBIN]

In KsGLWidget::_newCPUGraph() and in KsGLWidget::_newTaskGraph()
allocate KsPlot::Graph after getting data stream successfully.
Also remove unused "name" local variable in both functions.

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/KsGLWidget.cpp | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/src/KsGLWidget.cpp b/src/KsGLWidget.cpp
index 0a44e77..23485d2 100644
--- a/src/KsGLWidget.cpp
+++ b/src/KsGLWidget.cpp
@@ -794,13 +794,8 @@ void KsGLWidget::_makePluginShapes()
 
 KsPlot::Graph *KsGLWidget::_newCPUGraph(int sd, int cpu)
 {
-	QString name;
-	/* The CPU graph needs to know only the colors of the tasks. */
-	KsPlot::Graph *graph = new KsPlot::Graph(_model.histo(),
-						 &_pidColors,
-						 &_pidColors);
-
-	kshark_context *kshark_ctx(nullptr);
+	KsPlot::Graph *graph = nullptr;
+	kshark_context *kshark_ctx = nullptr;
 	kshark_data_stream *stream;
 	kshark_entry_collection *col;
 
@@ -811,6 +806,8 @@ KsPlot::Graph *KsGLWidget::_newCPUGraph(int sd, int cpu)
 	if (!stream)
 		return nullptr;
 
+	/* The CPU graph needs to know only the colors of the tasks. */
+	graph = new KsPlot::Graph(_model.histo(), &_pidColors, &_pidColors);
 	graph->setIdleSuppressed(true, stream->idle_pid);
 	graph->setHeight(KS_GRAPH_HEIGHT);
 	graph->setLabelText(KsUtils::cpuPlotName(cpu).toStdString());
@@ -827,15 +824,8 @@ KsPlot::Graph *KsGLWidget::_newCPUGraph(int sd, int cpu)
 
 KsPlot::Graph *KsGLWidget::_newTaskGraph(int sd, int pid)
 {
-	QString name;
-	/*
-	 * The Task graph needs to know the colors of the tasks and the colors
-	 * of the CPUs.
-	 */
-	KsPlot::Graph *graph = new KsPlot::Graph(_model.histo(),
-						 &_pidColors,
-						 &_cpuColors);
-	kshark_context *kshark_ctx(nullptr);
+	KsPlot::Graph *graph = nullptr;
+	kshark_context *kshark_ctx = nullptr;
 	kshark_entry_collection *col;
 	kshark_data_stream *stream;
 
@@ -846,6 +836,9 @@ KsPlot::Graph *KsGLWidget::_newTaskGraph(int sd, int pid)
 	if (!stream)
 		return nullptr;
 
+	/* The Task graph needs to know the colors of the tasks and the colors
+	 * of the CPUs */
+	graph = new KsPlot::Graph(_model.histo(), &_pidColors, &_cpuColors);
 	graph->setHeight(KS_GRAPH_HEIGHT);
 	graph->setLabelText(KsUtils::taskPlotName(sd, pid).toStdString());
 
-- 
2.43.0

[PATCH 08/34] kernelshark: Use lambda parameter instead of captured local variable

[Benjamin ROBIN]

In KsGLWidget::_find(), fix lamFindEntryByCPU() and lamFindEntryByPid()
which had a parameter "b": use it instead of using captured "bin"
local variable.

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/KsGLWidget.cpp | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/KsGLWidget.cpp b/src/KsGLWidget.cpp
index 23485d2..dd9f41a 100644
--- a/src/KsGLWidget.cpp
+++ b/src/KsGLWidget.cpp
@@ -986,15 +986,15 @@ bool KsGLWidget::_find(int bin, int sd, int cpu, int pid,
 		 * The click is over the CPU graphs. First try the exact
 		 * match.
 		 */
-		if (lamGetEntryByCPU(bin))
+		if (lamGetEntryByCPU(b))
 			return true;
 
 		/* Now look for a match, nearby the position of the click. */
 		for (int i = 1; i < variance; ++i) {
-			if (bin + i <= hSize && lamGetEntryByCPU(bin + i))
+			if (b + i <= hSize && lamGetEntryByCPU(b + i))
 				return true;
 
-			if (bin - i >= 0 && lamGetEntryByCPU(bin - i))
+			if (b - i >= 0 && lamGetEntryByCPU(b - i))
 				return true;
 		}
 
@@ -1007,15 +1007,15 @@ bool KsGLWidget::_find(int bin, int sd, int cpu, int pid,
 		 * The click is over the Task graphs. First try the exact
 		 * match.
 		 */
-		if (lamGetEntryByPid(bin))
+		if (lamGetEntryByPid(b))
 			return true;
 
 		/* Now look for a match, nearby the position of the click. */
 		for (int i = 1; i < variance; ++i) {
-			if ((bin + i <= hSize) && lamGetEntryByPid(bin + i))
+			if ((b + i <= hSize) && lamGetEntryByPid(b + i))
 				return true;
 
-			if ((bin - i >= 0) && lamGetEntryByPid(bin - i))
+			if ((b - i >= 0) && lamGetEntryByPid(b - i))
 				return true;
 		}
 
-- 
2.43.0

[PATCH 09/34] kernelshark: Keep overridden method protected instead of public

[Benjamin ROBIN]

Also add "override" keyword when missing.

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/KsGLWidget.hpp    | 41 +++++++++++++++++++++--------------------
 src/KsModels.hpp      |  9 +++++----
 src/KsPlotTools.hpp   |  2 +-
 src/KsTraceViewer.hpp |  9 +++++----
 src/KsWidgetsLib.hpp  |  9 +++++----
 5 files changed, 37 insertions(+), 33 deletions(-)

diff --git a/src/KsGLWidget.hpp b/src/KsGLWidget.hpp
index 1c6253f..cafc70b 100644
--- a/src/KsGLWidget.hpp
+++ b/src/KsGLWidget.hpp
@@ -75,12 +75,6 @@ public:
 
 	~KsGLWidget();
 
-	void initializeGL() override;
-
-	void resizeGL(int w, int h) override;
-
-	void paintGL() override;
-
 	void render();
 
 	void reset();
@@ -88,20 +82,6 @@ public:
 	/** Reprocess all graphs. */
 	void updateGeom() {resizeGL(width(), height());}
 
-	void mousePressEvent(QMouseEvent *event);
-
-	void mouseMoveEvent(QMouseEvent *event);
-
-	void mouseReleaseEvent(QMouseEvent *event);
-
-	void mouseDoubleClickEvent(QMouseEvent *event);
-
-	void wheelEvent(QWheelEvent * event);
-
-	void keyPressEvent(QKeyEvent *event);
-
-	void keyReleaseEvent(QKeyEvent *event);
-
 	void loadData(KsDataStore *data, bool resetPlots);
 
 	void loadColors();
@@ -214,6 +194,27 @@ public:
 	/** Free the list of plugin-defined shapes. */
 	void freePluginShapes();
 
+protected:
+	void initializeGL() override;
+
+	void resizeGL(int w, int h) override;
+
+	void paintGL() override;
+
+	void mousePressEvent(QMouseEvent *event) override;
+
+	void mouseMoveEvent(QMouseEvent *event) override;
+
+	void mouseReleaseEvent(QMouseEvent *event) override;
+
+	void mouseDoubleClickEvent(QMouseEvent *event) override;
+
+	void wheelEvent(QWheelEvent * event) override;
+
+	void keyPressEvent(QKeyEvent *event) override;
+
+	void keyReleaseEvent(QKeyEvent *event) override;
+
 signals:
 	/**
 	 * This signal is emitted when the mouse moves over a visible
diff --git a/src/KsModels.hpp b/src/KsModels.hpp
index 2883ce3..e113cf5 100644
--- a/src/KsModels.hpp
+++ b/src/KsModels.hpp
@@ -172,9 +172,6 @@ class KsFilterProxyModel : public QSortFilterProxyModel
 public:
 	explicit KsFilterProxyModel(QObject *parent = nullptr);
 
-	bool filterAcceptsRow(int sourceRow,
-			      const QModelIndex &sourceParent) const override;
-
 	void fill(KsDataStore *data);
 
 	void setSource(KsViewModel *s);
@@ -224,9 +221,13 @@ public:
 	/** A mutex used by the condition variable. */
 	std::mutex		_mutex;
 
-	/** A flag used to stop the serch for all threads. */
+	/** A flag used to stop the search for all threads. */
 	bool			_searchStop;
 
+protected:
+	bool filterAcceptsRow(int sourceRow,
+			      const QModelIndex &sourceParent) const override;
+
 private:
 	int			_searchProgress;
 
diff --git a/src/KsPlotTools.hpp b/src/KsPlotTools.hpp
index 68bd6f7..8f53077 100644
--- a/src/KsPlotTools.hpp
+++ b/src/KsPlotTools.hpp
@@ -662,7 +662,7 @@ private:
 	/** The vertical size (height) of the graphical element. */
 	int _height;
 
-	void _draw(const Color &col, float size) const;
+	void _draw(const Color &col, float size) const override;
 };
 
 }; // KsPlot
diff --git a/src/KsTraceViewer.hpp b/src/KsTraceViewer.hpp
index 99e4d38..7cc5751 100644
--- a/src/KsTraceViewer.hpp
+++ b/src/KsTraceViewer.hpp
@@ -79,10 +79,6 @@ public:
 
 	void setTopRow(size_t r);
 
-	void resizeEvent(QResizeEvent* event) override;
-
-	void keyReleaseEvent(QKeyEvent *event);
-
 	void markSwitch();
 
 	void showRow(size_t r, bool mark);
@@ -101,6 +97,11 @@ public:
 		_model.loadColors();
 	}
 
+protected:
+	void resizeEvent(QResizeEvent* event) override;
+
+	void keyReleaseEvent(QKeyEvent *event) override;
+
 signals:
 	/** Signal emitted when new row is selected. */
 	void select(size_t);
diff --git a/src/KsWidgetsLib.hpp b/src/KsWidgetsLib.hpp
index cc2dc99..e21441d 100644
--- a/src/KsWidgetsLib.hpp
+++ b/src/KsWidgetsLib.hpp
@@ -261,20 +261,21 @@ public:
 	 */
 	int sd() const {return _sd;}
 
+	/** The user provided an input. The widget has been modified. */
+	bool _userInput;
+
+protected:
 	/**
 	 * Reimplemented event handler used to update the geometry of the widget on
 	 * resize events.
 	 */
-	void resizeEvent(QResizeEvent* event)
+	void resizeEvent(QResizeEvent* event) override
 	{
 		KsUtils::setElidedText(&_streamLabel, _streamName,
 				       Qt::ElideLeft, width());
 		QApplication::processEvents();
 	}
 
-	/** The user provided an input. The widget has been modified. */
-	bool _userInput;
-
 protected:
 	QToolBar _tb;
 
-- 
2.43.0

[PATCH 10/34] kernelshark: Use sliced() or first() instead of mid/right/left()

[Benjamin ROBIN]

Fix Clazy warning and it is faster.

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/KsMainWindow.cpp | 2 +-
 src/KsUtils.cpp      | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/KsMainWindow.cpp b/src/KsMainWindow.cpp
index 68853d6..72ad371 100644
--- a/src/KsMainWindow.cpp
+++ b/src/KsMainWindow.cpp
@@ -1275,7 +1275,7 @@ void KsMainWindow::_load(const QString& fileName, bool append)
 		pbLabel += fileName;
 	} else {
 		pbLabel += "...";
-		pbLabel += fileName.mid(fileName.size() - 37, 37);
+		pbLabel += fileName.sliced(fileName.size() - 37);
 	}
 
 	setWindowTitle("Kernel Shark");
diff --git a/src/KsUtils.cpp b/src/KsUtils.cpp
index a68e4c1..b84add4 100644
--- a/src/KsUtils.cpp
+++ b/src/KsUtils.cpp
@@ -534,8 +534,8 @@ QVector<int> parseIdList(QString v_str)
 		int i = item.indexOf('-');
 		if (i > 0) {
 			/* This item is an interval. */
-			int to = item.right(item.size() - i - 1).toInt();
-			int from = item.left(i).toInt();
+			int to = item.sliced(i + 1).toInt();
+			int from = item.first(i).toInt();
 			int s = v.size();
 
 			v.resize(s + to - from + 1);
-- 
2.43.0

[PATCH 11/34] kernelshark: Prevent potential divide by zero in Shape::center()

[Benjamin ROBIN]

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/KsPlotTools.cpp | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/KsPlotTools.cpp b/src/KsPlotTools.cpp
index 1d63a9b..8011329 100644
--- a/src/KsPlotTools.cpp
+++ b/src/KsPlotTools.cpp
@@ -318,13 +318,15 @@ ksplot_point Shape::center() const
 {
 	ksplot_point c = {0, 0};
 
-	for (size_t i = 0; i < _nPoints; ++i) {
-		c.x += _points[i].x;
-		c.y += _points[i].y;
-	}
+	if (_nPoints > 0) {
+		for (size_t i = 0; i < _nPoints; ++i) {
+			c.x += _points[i].x;
+			c.y += _points[i].y;
+		}
 
-	c.x /= _nPoints;
-	c.y /= _nPoints;
+		c.x /= _nPoints;
+		c.y /= _nPoints;
+	}
 
 	return c;
 }
-- 
2.43.0

Re: [PATCH 11/34] kernelshark: Prevent potential divide by zero in Shape::center()

[Yordan Karadzhov]

On 1/14/24 19:17, Benjamin ROBIN wrote:
> Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
> ---
>   src/KsPlotTools.cpp | 14 ++++++++------
>   1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/src/KsPlotTools.cpp b/src/KsPlotTools.cpp
> index 1d63a9b..8011329 100644
> --- a/src/KsPlotTools.cpp
> +++ b/src/KsPlotTools.cpp
> @@ -318,13 +318,15 @@ ksplot_point Shape::center() const
>   {
>   	ksplot_point c = {0, 0};
>   
> -	for (size_t i = 0; i < _nPoints; ++i) {
> -		c.x += _points[i].x;
> -		c.y += _points[i].y;
> -	}
> +	if (_nPoints > 0) {

Can we just have

if (_nPoints == 0)
	return c;


Thanks,
Y.

> +		for (size_t i = 0; i < _nPoints; ++i) {
> +			c.x += _points[i].x;
> +			c.y += _points[i].y;
> +		}
>   
> -	c.x /= _nPoints;
> -	c.y /= _nPoints;
> +		c.x /= _nPoints;
> +		c.y /= _nPoints;
> +	}
>   
>   	return c;
>   }

[PATCH v2 11/34] kernelshark: Prevent potential divide by zero in Shape::center()

[Benjamin ROBIN]

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/KsPlotTools.cpp | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/KsPlotTools.cpp b/src/KsPlotTools.cpp
index 1d63a9b..f362eaa 100644
--- a/src/KsPlotTools.cpp
+++ b/src/KsPlotTools.cpp
@@ -318,6 +318,9 @@ ksplot_point Shape::center() const
 {
 	ksplot_point c = {0, 0};
 
+	if (_nPoints == 0)
+		return c;
+
 	for (size_t i = 0; i < _nPoints; ++i) {
 		c.x += _points[i].x;
 		c.y += _points[i].y;
-- 
2.43.0

[PATCH 12/34] kernelshark: Fix potential access to uninitialized variable

[Benjamin ROBIN]

If "nStreams" is zero, jstream may be accessed while uninitialized
in KsSession::_savePlots() and in KsSession::_getPlots()

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/KsSession.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/KsSession.cpp b/src/KsSession.cpp
index b9edb3a..af86281 100644
--- a/src/KsSession.cpp
+++ b/src/KsSession.cpp
@@ -348,7 +348,7 @@ void KsSession::loadGraphs(kshark_context *kshark_ctx,
 void KsSession::_savePlots(int sd, KsGLWidget *glw, bool cpu)
 {
 	kshark_config_doc *streamsConf = kshark_config_alloc(KS_CONFIG_JSON);
-	json_object *jallStreams, *jstream, *jstreamId, *jplots;
+	json_object *jallStreams, *jstream = nullptr, *jstreamId, *jplots;
 	QVector<int> plotIds;
 	int nStreams;
 
@@ -438,7 +438,7 @@ void KsSession::_saveComboPlots(KsGLWidget *glw)
 QVector<int> KsSession::_getPlots(int sd, bool cpu)
 {
 	kshark_config_doc *streamsConf = kshark_config_alloc(KS_CONFIG_JSON);
-	json_object *jallStreams, *jstream, *jstreamId, *jplots;
+	json_object *jallStreams, *jstream = nullptr, *jstreamId, *jplots;
 	int nStreams, nPlots, id;
 	const char *plotKey;
 	QVector<int> plots;
-- 
2.43.0

[PATCH 13/34] kernelshark: Remove unused locals variables

[Benjamin ROBIN]

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/KsTraceGraph.cpp           | 1 -
 src/KsUtils.cpp                | 1 -
 src/plugins/KVMComboDialog.cpp | 1 -
 3 files changed, 3 deletions(-)

diff --git a/src/KsTraceGraph.cpp b/src/KsTraceGraph.cpp
index f80477d..bc910c8 100644
--- a/src/KsTraceGraph.cpp
+++ b/src/KsTraceGraph.cpp
@@ -331,7 +331,6 @@ void KsTraceGraph::_setPointerInfo(size_t i)
 	QString aux(lanMakeString(kshark_get_aux_info(e)));
 	QString info(lanMakeString(kshark_get_info(e)));
 	QString comm(lanMakeString(kshark_get_task(e)));
-	QString elidedText;
 	int labelWidth;
 	uint64_t sec, usec;
 	char *pointer;
diff --git a/src/KsUtils.cpp b/src/KsUtils.cpp
index b84add4..fc1fc52 100644
--- a/src/KsUtils.cpp
+++ b/src/KsUtils.cpp
@@ -1414,7 +1414,6 @@ void KsPluginManager::updatePlugins(int sd, QVector<int> pluginStates)
 	kshark_context *kshark_ctx(nullptr);
 	kshark_data_stream *stream;
 	kshark_dpi_list *plugin;
-	QVector<int> vec;
 	int i(0);
 
 	if (!kshark_instance(&kshark_ctx))
diff --git a/src/plugins/KVMComboDialog.cpp b/src/plugins/KVMComboDialog.cpp
index 6be68d4..99113ed 100644
--- a/src/plugins/KVMComboDialog.cpp
+++ b/src/plugins/KVMComboDialog.cpp
@@ -342,7 +342,6 @@ void KsComboPlotDialog::_guestStreamChanged(int)
 		return;
 
 	int newGuestId = _guestStreamComboBox.currentData().toInt();
-	QVector<int> vcpuCBs(_guestMapCount, 0);
 
 	_plotMap[_currentGuestStream] = _streamCombos(_currentGuestStream);
 
-- 
2.43.0

[PATCH 14/34] kernelshark: Fix range-loop-reference Clazy warning

[Benjamin ROBIN]

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/KsUtils.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/KsUtils.cpp b/src/KsUtils.cpp
index fc1fc52..83f8f2e 100644
--- a/src/KsUtils.cpp
+++ b/src/KsUtils.cpp
@@ -530,7 +530,7 @@ QVector<int> parseIdList(QString v_str)
 	QStringList list = v_str.split(",", KS_SPLIT_SkipEmptyParts);
 	QVector<int> v;
 
-	for (auto item: list) {
+	for (const auto &item: list) {
 		int i = item.indexOf('-');
 		if (i > 0) {
 			/* This item is an interval. */
-- 
2.43.0

[PATCH 15/34] kernelshark: Fix moving a temp object prevents copy elision warning

[Benjamin ROBIN]

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/KsTraceViewer.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/KsTraceViewer.cpp b/src/KsTraceViewer.cpp
index d4c9dd9..5eec312 100644
--- a/src/KsTraceViewer.cpp
+++ b/src/KsTraceViewer.cpp
@@ -855,7 +855,7 @@ void KsTraceViewer::_searchItemsMT()
 
 	QVector<QList<int>> res;
 	for (auto &m: maps)
-		res.append(std::move(m.get()));
+		res.append(m.get());
 
 	lamSearchMerge(_matchList, res);
 }
-- 
2.43.0

[PATCH 16/34] kernelshark: Add receiver object to connect() call

[Benjamin ROBIN]

When calling a lambda, this is recommended to pass the receiver object

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/KsTraceViewer.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/KsTraceViewer.cpp b/src/KsTraceViewer.cpp
index 5eec312..5e8e63f 100644
--- a/src/KsTraceViewer.cpp
+++ b/src/KsTraceViewer.cpp
@@ -139,7 +139,7 @@ KsTraceViewer::KsTraceViewer(QWidget *parent)
 		}
 	};
 	connect(&_selectionModel,	&QItemSelectionModel::selectionChanged,
-		lamSelectionChanged);
+		this,			lamSelectionChanged);
 
 	_searchFSM.placeInToolBar(&_toolbar);
 
-- 
2.43.0

[PATCH 17/34] kernelshark: Return by reference the list of header in KsModels

[Benjamin ROBIN]

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/KsModels.hpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/KsModels.hpp b/src/KsModels.hpp
index e113cf5..b95c6c1 100644
--- a/src/KsModels.hpp
+++ b/src/KsModels.hpp
@@ -81,7 +81,7 @@ public:
 	void update(KsDataStore *data);
 
 	/** Get the list of column's headers. */
-	QStringList header() const {return _header;}
+	const QStringList& header() const {return _header;}
 
 	QString getValueStr(int column, int row) const;
 
-- 
2.43.0

[PATCH 18/34] kernelshark: Fix detaching-temporary Clazy warning

[Benjamin ROBIN]

Use QList::at() instead of QList::operator[]()

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/KsTraceViewer.cpp | 4 ++--
 src/KsUtils.hpp       | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/KsTraceViewer.cpp b/src/KsTraceViewer.cpp
index 5e8e63f..86e9185 100644
--- a/src/KsTraceViewer.cpp
+++ b/src/KsTraceViewer.cpp
@@ -624,11 +624,11 @@ void KsTraceViewer::_resizeToContents()
 	 * column by hand.
 	 */
 	col = KsViewModel::TRACE_VIEW_COL_STREAM;
-	columnSize = STRING_WIDTH(_model.header()[col]) + FONT_WIDTH;
+	columnSize = STRING_WIDTH(_model.header().at(col)) + FONT_WIDTH;
 	_view.setColumnWidth(col, columnSize);
 
 	col = KsViewModel::TRACE_VIEW_COL_CPU;
-	columnSize = STRING_WIDTH(_model.header()[col]) + FONT_WIDTH * 2;
+	columnSize = STRING_WIDTH(_model.header().at(col)) + FONT_WIDTH * 2;
 	_view.setColumnWidth(col, columnSize);
 
 	col = KsViewModel::TRACE_VIEW_COL_INDEX;
diff --git a/src/KsUtils.hpp b/src/KsUtils.hpp
index 85f81c3..67af99a 100644
--- a/src/KsUtils.hpp
+++ b/src/KsUtils.hpp
@@ -25,10 +25,10 @@
 #include "KsPlotTools.hpp"
 
 /** Macro providing the height of the screen in pixels. */
-#define SCREEN_HEIGHT  QGuiApplication::screens()[0]->geometry().height()
+#define SCREEN_HEIGHT  QGuiApplication::screens().at(0)->geometry().height()
 
 /** Macro providing the width of the screen in pixels. */
-#define SCREEN_WIDTH   QGuiApplication::screens()[0]->geometry().width()
+#define SCREEN_WIDTH   QGuiApplication::screens().at(0)->geometry().width()
 
 //! @cond Doxygen_Suppress
 
-- 
2.43.0

[PATCH 19/34] kernelshark: Fix qfileinfo-exists Clazy warning

[Benjamin ROBIN]

Use static QFileInfo::exists() instead

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/KsUtils.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/KsUtils.cpp b/src/KsUtils.cpp
index 83f8f2e..792678d 100644
--- a/src/KsUtils.cpp
+++ b/src/KsUtils.cpp
@@ -471,7 +471,7 @@ QString getSaveFile(QWidget *parent,
 	if (!fileName.isEmpty() && !fileName.endsWith(extension)) {
 		fileName += extension;
 
-		if (QFileInfo(fileName).exists()) {
+		if (QFileInfo::exists(fileName)) {
 			if (!KsWidgetsLib::fileExistsDialog(fileName))
 				fileName.clear();
 		}
-- 
2.43.0

[PATCH 20/34] kernelshark: Fix potential memory leaks in libkshark-configio

[Benjamin ROBIN]

Allocate a new kshark_config_doc only if the format is supported.

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/libkshark-configio.c | 74 ++++++++++++++++++++++------------------
 1 file changed, 41 insertions(+), 33 deletions(-)

diff --git a/src/libkshark-configio.c b/src/libkshark-configio.c
index 9a1ba60..853e056 100644
--- a/src/libkshark-configio.c
+++ b/src/libkshark-configio.c
@@ -675,23 +675,25 @@ struct kshark_config_doc *
 kshark_export_plugin_file(struct kshark_plugin_list *plugin,
 			  enum kshark_config_formats format)
 {
-	/*  Create a new Configuration document. */
-	struct kshark_config_doc *conf =
-		kshark_config_new("kshark.config.library", format);
-
-	if (!conf)
-		return NULL;
+	struct kshark_config_doc *conf;
 
 	switch (format) {
 	case KS_CONFIG_JSON:
-		kshark_plugin_to_json(plugin, conf->conf_doc);
-		return conf;
+		break;
 
 	default:
 		fprintf(stderr, "Document format %d not supported\n",
-			conf->format);
+			format);
 		return NULL;
 	}
+
+	/*  Create a new Configuration document. */
+	conf = kshark_config_new("kshark.config.library", format);
+	if (!conf)
+		return NULL;
+
+	kshark_plugin_to_json(plugin, conf->conf_doc);
+	return conf;
 }
 
 static bool kshark_all_plugins_to_json(struct kshark_context *kshark_ctx,
@@ -739,22 +741,24 @@ struct kshark_config_doc *
 kshark_export_all_plugins(struct kshark_context *kshark_ctx,
 			  enum kshark_config_formats format)
 {
-	struct kshark_config_doc *conf =
-		kshark_config_new("kshark.config.plugins", KS_CONFIG_JSON);
-
-	if (!conf)
-		return NULL;
+	struct kshark_config_doc *conf;
 
 	switch (format) {
 	case KS_CONFIG_JSON:
-		kshark_all_plugins_to_json(kshark_ctx, conf->conf_doc);
-		return conf;
+		break;
 
 	default:
 		fprintf(stderr, "Document format %d not supported\n",
-			conf->format);
+			format);
 		return NULL;
 	}
+
+	conf = kshark_config_new("kshark.config.plugins", format);
+	if (!conf)
+		return NULL;
+
+	kshark_all_plugins_to_json(kshark_ctx, conf->conf_doc);
+	return conf;
 }
 
 static bool kshark_plugin_from_json(struct kshark_context *kshark_ctx,
@@ -867,22 +871,24 @@ struct kshark_config_doc *
 kshark_export_stream_plugins(struct kshark_data_stream *stream,
 			     enum kshark_config_formats format)
 {
-	struct kshark_config_doc *conf =
-		kshark_config_new("kshark.config.plugins", KS_CONFIG_JSON);
-
-	if (!conf)
-		return NULL;
+	struct kshark_config_doc *conf;
 
 	switch (format) {
 	case KS_CONFIG_JSON:
-		kshark_stream_plugins_to_json(stream, conf->conf_doc);
-		return conf;
+		break;
 
 	default:
 		fprintf(stderr, "Document format %d not supported\n",
-			conf->format);
+			format);
 		return NULL;
 	}
+
+	conf = kshark_config_new("kshark.config.plugins", KS_CONFIG_JSON);
+	if (!conf)
+		return NULL;
+
+	kshark_stream_plugins_to_json(stream, conf->conf_doc);
+	return conf;
 }
 
 static bool kshark_stream_plugins_from_json(struct kshark_context *kshark_ctx,
@@ -1019,23 +1025,25 @@ struct kshark_config_doc *
 kshark_export_model(struct kshark_trace_histo *histo,
 		    enum kshark_config_formats format)
 {
-	/*  Create a new Configuration document. */
-	struct kshark_config_doc *conf =
-		kshark_config_new("kshark.config.model", format);
-
-	if (!conf)
-		return NULL;
+	struct kshark_config_doc *conf;
 
 	switch (format) {
 	case KS_CONFIG_JSON:
-		kshark_model_to_json(histo, conf->conf_doc);
-		return conf;
+		break;
 
 	default:
 		fprintf(stderr, "Document format %d not supported\n",
 			format);
 		return NULL;
 	}
+
+	/*  Create a new Configuration document. */
+	conf = kshark_config_new("kshark.config.model", format);
+	if (!conf)
+		return NULL;
+
+	kshark_model_to_json(histo, conf->conf_doc);
+	return conf;
 }
 
 static bool kshark_model_from_json(struct kshark_trace_histo *histo,
-- 
2.43.0

Re: [PATCH 20/34] kernelshark: Fix potential memory leaks in libkshark-configio

[Yordan Karadzhov]

On 1/14/24 19:17, Benjamin ROBIN wrote:
> Allocate a new kshark_config_doc only if the format is supported.
> 
> Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
> ---
>   src/libkshark-configio.c | 74 ++++++++++++++++++++++------------------
>   1 file changed, 41 insertions(+), 33 deletions(-)
> 
> diff --git a/src/libkshark-configio.c b/src/libkshark-configio.c
> index 9a1ba60..853e056 100644
> --- a/src/libkshark-configio.c
> +++ b/src/libkshark-configio.c
> @@ -675,23 +675,25 @@ struct kshark_config_doc *
>   kshark_export_plugin_file(struct kshark_plugin_list *plugin,
>   			  enum kshark_config_formats format)
>   {
> -	/*  Create a new Configuration document. */
> -	struct kshark_config_doc *conf =
> -		kshark_config_new("kshark.config.library", format);
> -
> -	if (!conf)
> -		return NULL;
> +	struct kshark_config_doc *conf;
>   
>   	switch (format) {
>   	case KS_CONFIG_JSON:
> -		kshark_plugin_to_json(plugin, conf->conf_doc);
> -		return conf;
> +		break;
>   
>   	default:
>   		fprintf(stderr, "Document format %d not supported\n",
> -			conf->format);
> +			format);
>   		return NULL;
>   	}
> +
> +	/*  Create a new Configuration document. */
> +	conf = kshark_config_new("kshark.config.library", format);
> +	if (!conf)
> +		return NULL;
> +
> +	kshark_plugin_to_json(plugin, conf->conf_doc);
> +	return conf;
>   }
>   
>   static bool kshark_all_plugins_to_json(struct kshark_context *kshark_ctx,
> @@ -739,22 +741,24 @@ struct kshark_config_doc *
>   kshark_export_all_plugins(struct kshark_context *kshark_ctx,
>   			  enum kshark_config_formats format)
>   {
> -	struct kshark_config_doc *conf =
> -		kshark_config_new("kshark.config.plugins", KS_CONFIG_JSON);
> -
> -	if (!conf)
> -		return NULL;
> +	struct kshark_config_doc *conf;
>   
>   	switch (format) {
>   	case KS_CONFIG_JSON:
> -		kshark_all_plugins_to_json(kshark_ctx, conf->conf_doc);
> -		return conf;

I agree that this code is a bit confusing. Long time ago we had the
idea that we will support not only 'json' but also other formats. And
that we will have other 'kshark_plugin_to_xxx()' APIs. This is the
reason to have this 'switch'.

Please keep the code as it is and just call kshark_free_config_doc()
in the default (error) case.

> +		break;
>   
>   	default:
>   		fprintf(stderr, "Document format %d not supported\n",
> -			conf->format);
> +			format);
>   		return NULL;
>   	}
> +
> +	conf = kshark_config_new("kshark.config.plugins", format);
> +	if (!conf)
> +		return NULL;
> +
> +	kshark_all_plugins_to_json(kshark_ctx, conf->conf_doc);
> +	return conf;
>   }
>   
>   static bool kshark_plugin_from_json(struct kshark_context *kshark_ctx,
> @@ -867,22 +871,24 @@ struct kshark_config_doc *
>   kshark_export_stream_plugins(struct kshark_data_stream *stream,
>   			     enum kshark_config_formats format)
>   {
> -	struct kshark_config_doc *conf =
> -		kshark_config_new("kshark.config.plugins", KS_CONFIG_JSON);
> -
> -	if (!conf)
> -		return NULL;
> +	struct kshark_config_doc *conf;
>   
>   	switch (format) {
>   	case KS_CONFIG_JSON:
> -		kshark_stream_plugins_to_json(stream, conf->conf_doc);
> -		return conf;

Same comment applies here.
Thanks!
Y.

> +		break;
>   
>   	default:
>   		fprintf(stderr, "Document format %d not supported\n",
> -			conf->format);
> +			format);
>   		return NULL;
>   	}
> +
> +	conf = kshark_config_new("kshark.config.plugins", KS_CONFIG_JSON);
> +	if (!conf)
> +		return NULL;
> +
> +	kshark_stream_plugins_to_json(stream, conf->conf_doc);
> +	return conf;
>   }
>   
>   static bool kshark_stream_plugins_from_json(struct kshark_context *kshark_ctx,
> @@ -1019,23 +1025,25 @@ struct kshark_config_doc *
>   kshark_export_model(struct kshark_trace_histo *histo,
>   		    enum kshark_config_formats format)
>   {
> -	/*  Create a new Configuration document. */
> -	struct kshark_config_doc *conf =
> -		kshark_config_new("kshark.config.model", format);
> -
> -	if (!conf)
> -		return NULL;
> +	struct kshark_config_doc *conf;
>   
>   	switch (format) {
>   	case KS_CONFIG_JSON:
> -		kshark_model_to_json(histo, conf->conf_doc);
> -		return conf;
> +		break;
>   
>   	default:
>   		fprintf(stderr, "Document format %d not supported\n",
>   			format);
>   		return NULL;
>   	}
> +
> +	/*  Create a new Configuration document. */
> +	conf = kshark_config_new("kshark.config.model", format);
> +	if (!conf)
> +		return NULL;
> +
> +	kshark_model_to_json(histo, conf->conf_doc);
> +	return conf;
>   }
>   
>   static bool kshark_model_from_json(struct kshark_trace_histo *histo,

[PATCH v2 20/34] kernelshark: Fix potential memory leaks in libkshark-configio

[Benjamin ROBIN]

Free previously allocated kshark_config_doc if format is not supported.

Also allow to call kshark_export_*() functions that allocate a new
kshark_config_doc with format equal to KS_CONFIG_AUTO.

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/libkshark-configio.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/src/libkshark-configio.c b/src/libkshark-configio.c
index 9a1ba60..3a0e979 100644
--- a/src/libkshark-configio.c
+++ b/src/libkshark-configio.c
@@ -515,7 +515,7 @@ kshark_export_trace_file(const char *file, const char *name,
 	if (!conf)
 		return NULL;
 
-	switch (format) {
+	switch (conf->format) {
 	case KS_CONFIG_JSON:
 		kshark_trace_file_to_json(file, name, conf->conf_doc);
 		return conf;
@@ -523,6 +523,7 @@ kshark_export_trace_file(const char *file, const char *name,
 	default:
 		fprintf(stderr, "Document format %d not supported\n",
 			conf->format);
+		kshark_free_config_doc(conf);
 		return NULL;
 	}
 }
@@ -682,7 +683,7 @@ kshark_export_plugin_file(struct kshark_plugin_list *plugin,
 	if (!conf)
 		return NULL;
 
-	switch (format) {
+	switch (conf->format) {
 	case KS_CONFIG_JSON:
 		kshark_plugin_to_json(plugin, conf->conf_doc);
 		return conf;
@@ -690,6 +691,7 @@ kshark_export_plugin_file(struct kshark_plugin_list *plugin,
 	default:
 		fprintf(stderr, "Document format %d not supported\n",
 			conf->format);
+		kshark_free_config_doc(conf);
 		return NULL;
 	}
 }
@@ -740,12 +742,12 @@ kshark_export_all_plugins(struct kshark_context *kshark_ctx,
 			  enum kshark_config_formats format)
 {
 	struct kshark_config_doc *conf =
-		kshark_config_new("kshark.config.plugins", KS_CONFIG_JSON);
+		kshark_config_new("kshark.config.plugins", format);
 
 	if (!conf)
 		return NULL;
 
-	switch (format) {
+	switch (conf->format) {
 	case KS_CONFIG_JSON:
 		kshark_all_plugins_to_json(kshark_ctx, conf->conf_doc);
 		return conf;
@@ -753,6 +755,7 @@ kshark_export_all_plugins(struct kshark_context *kshark_ctx,
 	default:
 		fprintf(stderr, "Document format %d not supported\n",
 			conf->format);
+		kshark_free_config_doc(conf);
 		return NULL;
 	}
 }
@@ -868,12 +871,12 @@ kshark_export_stream_plugins(struct kshark_data_stream *stream,
 			     enum kshark_config_formats format)
 {
 	struct kshark_config_doc *conf =
-		kshark_config_new("kshark.config.plugins", KS_CONFIG_JSON);
+		kshark_config_new("kshark.config.plugins", format);
 
 	if (!conf)
 		return NULL;
 
-	switch (format) {
+	switch (conf->format) {
 	case KS_CONFIG_JSON:
 		kshark_stream_plugins_to_json(stream, conf->conf_doc);
 		return conf;
@@ -881,6 +884,7 @@ kshark_export_stream_plugins(struct kshark_data_stream *stream,
 	default:
 		fprintf(stderr, "Document format %d not supported\n",
 			conf->format);
+		kshark_free_config_doc(conf);
 		return NULL;
 	}
 }
@@ -1026,14 +1030,15 @@ kshark_export_model(struct kshark_trace_histo *histo,
 	if (!conf)
 		return NULL;
 
-	switch (format) {
+	switch (conf->format) {
 	case KS_CONFIG_JSON:
 		kshark_model_to_json(histo, conf->conf_doc);
 		return conf;
 
 	default:
 		fprintf(stderr, "Document format %d not supported\n",
-			format);
+			conf->format);
+		kshark_free_config_doc(conf);
 		return NULL;
 	}
 }
-- 
2.43.0

[PATCH 21/34] kernelshark: Fix potential access to uninitialized variable

[Benjamin ROBIN]

Always call json_object_put() with a valid "jlist" pointer.

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/libkshark-configio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/libkshark-configio.c b/src/libkshark-configio.c
index 853e056..49f957b 100644
--- a/src/libkshark-configio.c
+++ b/src/libkshark-configio.c
@@ -777,7 +777,7 @@ static bool kshark_plugin_from_json(struct kshark_context *kshark_ctx,
 static bool kshark_all_plugins_from_json(struct kshark_context *kshark_ctx,
 					 struct json_object *jobj)
 {
-	struct json_object *jlist, *jfile;
+	struct json_object *jlist = NULL, *jfile = NULL;
 	int i, n_plugins;
 
 	if (!kshark_ctx || !jobj)
@@ -902,7 +902,7 @@ static bool kshark_stream_plugins_from_json(struct kshark_context *kshark_ctx,
 	int i, n_plugins;
 	bool active;
 
-	jplg = jname = jstatus = NULL;
+	jlist = jplg = jname = jstatus = NULL;
 
 	if (!kshark_ctx || !stream || !jobj)
 		return false;
-- 
2.43.0

[PATCH 22/34] kernelshark: Fix potential double free of histo->map, histo->bin_count

[Benjamin ROBIN]

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/libkshark-model.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/libkshark-model.c b/src/libkshark-model.c
index 44f829d..4cd9f6a 100644
--- a/src/libkshark-model.c
+++ b/src/libkshark-model.c
@@ -104,7 +104,8 @@ static void ksmodel_set_in_range_bining(struct kshark_trace_histo *histo,
 
 		free(histo->bin_count);
 		free(histo->map);
-
+		histo->map = NULL;
+		histo->bin_count = NULL;
 		return;
 	}
 
-- 
2.43.0

[PATCH 23/34] kernelshark: Fix 'const' type qualifier on return type has no effect

[Benjamin ROBIN]

Also fix clang build, which allow to execute Clang-Tidy and Clazy

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/libkshark-tepdata.c | 12 ++++++------
 src/libkshark.h         | 20 ++++++++++----------
 tests/test-input.c      |  4 ++--
 tests/test-input_ctrl.c |  4 ++--
 4 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/src/libkshark-tepdata.c b/src/libkshark-tepdata.c
index b780957..2d0fcb0 100644
--- a/src/libkshark-tepdata.c
+++ b/src/libkshark-tepdata.c
@@ -634,8 +634,8 @@ ssize_t kshark_load_tep_records(struct kshark_context *kshark_ctx, int sd,
 	return -ENOMEM;
 }
 
-static const int tepdata_get_event_id(struct kshark_data_stream *stream,
-				      const struct kshark_entry *entry)
+static int tepdata_get_event_id(struct kshark_data_stream *stream,
+				const struct kshark_entry *entry)
 {
 	int event_id = KS_EMPTY_BIN;
 	struct tep_record *record;
@@ -727,8 +727,8 @@ static char *tepdata_get_event_name(struct kshark_data_stream *stream,
 	return buffer;
 }
 
-static const int tepdata_get_pid(struct kshark_data_stream *stream,
-				 const struct kshark_entry *entry)
+static int tepdata_get_pid(struct kshark_data_stream *stream,
+			   const struct kshark_entry *entry)
 {
 	struct tep_record *record;
 	int pid = KS_EMPTY_BIN;
@@ -1054,8 +1054,8 @@ static char *tepdata_dump_entry(struct kshark_data_stream *stream,
 	return entry_str;
 }
 
-static const int tepdata_find_event_id(struct kshark_data_stream *stream,
-				       const char *event_name)
+static int tepdata_find_event_id(struct kshark_data_stream *stream,
+				 const char *event_name)
 {
 	struct tep_event *event;
 	char *buffer, *system, *name;
diff --git a/src/libkshark.h b/src/libkshark.h
index 1514f33..97d3227 100644
--- a/src/libkshark.h
+++ b/src/libkshark.h
@@ -142,8 +142,8 @@ typedef char *(*stream_get_str_func) (struct kshark_data_stream *,
 				      const struct kshark_entry *);
 
 /** A function type to be used by the method interface of the data stream. */
-typedef const int (*stream_get_int_func) (struct kshark_data_stream *,
-					  const struct kshark_entry *);
+typedef int (*stream_get_int_func) (struct kshark_data_stream *,
+				    const struct kshark_entry *);
 
 /** A function type to be used by the method interface of the data stream. */
 typedef int (*stream_find_id_func) (struct kshark_data_stream *,
@@ -176,16 +176,16 @@ typedef kshark_event_field_format
 			    const char *);
 
 /** A function type to be used by the method interface of the data stream. */
-typedef const int (*stream_read_event_field) (struct kshark_data_stream *,
-					      const struct kshark_entry *,
-					      const char *,
-					      int64_t *);
+typedef int (*stream_read_event_field) (struct kshark_data_stream *,
+					const struct kshark_entry *,
+					const char *,
+					int64_t *);
 
 /** A function type to be used by the method interface of the data stream. */
-typedef const int (*stream_read_record_field) (struct kshark_data_stream *,
-					       void *,
-					       const char *,
-					       int64_t *);
+typedef int (*stream_read_record_field) (struct kshark_data_stream *,
+					 void *,
+					 const char *,
+					 int64_t *);
 
 struct kshark_context;
 
diff --git a/tests/test-input.c b/tests/test-input.c
index 31620b9..c6a5fa2 100644
--- a/tests/test-input.c
+++ b/tests/test-input.c
@@ -69,8 +69,8 @@ bool KSHARK_INPUT_CHECK(const char *file, char **format)
 	return false;
 }
 
-static const int get_pid(struct kshark_data_stream *stream,
-			 const struct kshark_entry *entry)
+static int get_pid(struct kshark_data_stream *stream,
+		   const struct kshark_entry *entry)
 {
 	return entry->pid;
 }
diff --git a/tests/test-input_ctrl.c b/tests/test-input_ctrl.c
index 3dcc92e..77abab1 100644
--- a/tests/test-input_ctrl.c
+++ b/tests/test-input_ctrl.c
@@ -71,8 +71,8 @@ bool KSHARK_INPUT_CHECK(const char *file, char **format)
 	return false;
 }
 
-static const int get_pid(struct kshark_data_stream *stream,
-			 const struct kshark_entry *entry)
+static int get_pid(struct kshark_data_stream *stream,
+		   const struct kshark_entry *entry)
 {
 	return entry->pid;
 }
-- 
2.43.0

[PATCH 24/34] kernelshark: Fix potential memory leaks in libkshark-tepdata

[Benjamin ROBIN]

- In tepdata_get_field_names(), buffer was never free on error
- In kshark_tep_open_buffer(), names were never free if
  kshark_get_data_stream() failed
- In kshark_tep_open_buffer(), prevent any double free error with
  "name" and "file" fields of buffer_stream
- In kshark_tep_init_all_buffers(), return failure code if failed to
  copy "name" and "file" fields of buffer_stream

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/libkshark-tepdata.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/src/libkshark-tepdata.c b/src/libkshark-tepdata.c
index 2d0fcb0..d15c155 100644
--- a/src/libkshark-tepdata.c
+++ b/src/libkshark-tepdata.c
@@ -949,6 +949,7 @@ static int tepdata_get_field_names(struct kshark_data_stream *stream,
 	for (i = 0; i < nr_fields; ++i)
 		free(buffer[i]);
 
+	free(buffer);
 	return -EFAULT;
 }
 
@@ -1424,8 +1425,10 @@ int kshark_tep_open_buffer(struct kshark_context *kshark_ctx, int sd,
 
 	sd_buffer = kshark_add_stream(kshark_ctx);
 	buffer_stream = kshark_get_data_stream(kshark_ctx, sd_buffer);
-	if (!buffer_stream)
-		return -EFAULT;
+	if (!buffer_stream) {
+		ret = -EFAULT;
+		goto fail;
+	}
 
 	for (i = 0; i < n_buffers; ++i) {
 		if (strcmp(buffer_name, names[i]) == 0) {
@@ -1438,7 +1441,8 @@ int kshark_tep_open_buffer(struct kshark_context *kshark_ctx, int sd,
 			if (!buffer_stream->name || !buffer_stream->file) {
 				free(buffer_stream->name);
 				free(buffer_stream->file);
-
+				buffer_stream->name = NULL;
+				buffer_stream->file = NULL;
 				ret = -ENOMEM;
 				break;
 			}
@@ -1449,6 +1453,7 @@ int kshark_tep_open_buffer(struct kshark_context *kshark_ctx, int sd,
 		}
 	}
 
+fail:
 	for (i = 0; i < n_buffers; ++i)
 		free(names[i]);
 	free(names);
@@ -1500,8 +1505,9 @@ int kshark_tep_init_all_buffers(struct kshark_context *kshark_ctx,
 		if (!buffer_stream->name || !buffer_stream->file) {
 			free(buffer_stream->name);
 			free(buffer_stream->file);
-			ret = -ENOMEM;
-			break;
+			buffer_stream->name = NULL;
+			buffer_stream->file = NULL;
+			return -ENOMEM;
 		}
 
 		ret = kshark_tep_stream_init(buffer_stream, buffer_input);
-- 
2.43.0

Re: [PATCH 24/34] kernelshark: Fix potential memory leaks in libkshark-tepdata

[Yordan Karadzhov]

On 1/14/24 19:17, Benjamin ROBIN wrote:
> - In tepdata_get_field_names(), buffer was never free on error
> - In kshark_tep_open_buffer(), names were never free if
>    kshark_get_data_stream() failed
> - In kshark_tep_open_buffer(), prevent any double free error with
>    "name" and "file" fields of buffer_stream
> - In kshark_tep_init_all_buffers(), return failure code if failed to
>    copy "name" and "file" fields of buffer_stream
> 
> Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
> ---
>   src/libkshark-tepdata.c | 16 +++++++++++-----
>   1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/src/libkshark-tepdata.c b/src/libkshark-tepdata.c
> index 2d0fcb0..d15c155 100644
> --- a/src/libkshark-tepdata.c
> +++ b/src/libkshark-tepdata.c
> @@ -949,6 +949,7 @@ static int tepdata_get_field_names(struct kshark_data_stream *stream,
>   	for (i = 0; i < nr_fields; ++i)
>   		free(buffer[i]);
>   
> +	free(buffer);
>   	return -EFAULT;
>   }
>   
> @@ -1424,8 +1425,10 @@ int kshark_tep_open_buffer(struct kshark_context *kshark_ctx, int sd,
>   
>   	sd_buffer = kshark_add_stream(kshark_ctx);
>   	buffer_stream = kshark_get_data_stream(kshark_ctx, sd_buffer);
> -	if (!buffer_stream)
> -		return -EFAULT;
> +	if (!buffer_stream) {
> +		ret = -EFAULT;
> +		goto fail;
> +	}
>   
>   	for (i = 0; i < n_buffers; ++i) {
>   		if (strcmp(buffer_name, names[i]) == 0) {
> @@ -1438,7 +1441,8 @@ int kshark_tep_open_buffer(struct kshark_context *kshark_ctx, int sd,
>   			if (!buffer_stream->name || !buffer_stream->file) {
>   				free(buffer_stream->name);
>   				free(buffer_stream->file);
> -
> +				buffer_stream->name = NULL;
> +				buffer_stream->file = NULL;
>   				ret = -ENOMEM;
>   				break;
>   			}
> @@ -1449,6 +1453,7 @@ int kshark_tep_open_buffer(struct kshark_context *kshark_ctx, int sd,
>   		}
>   	}
>   
> +fail:

This is not a true 'fail' because the code below gets executed even
if everything is fine. Perhaps you can use "free" or "end" or something
similar?

Thanks,
Y.

>   	for (i = 0; i < n_buffers; ++i)
>   		free(names[i]);
>   	free(names);
> @@ -1500,8 +1505,9 @@ int kshark_tep_init_all_buffers(struct kshark_context *kshark_ctx,
>   		if (!buffer_stream->name || !buffer_stream->file) {
>   			free(buffer_stream->name);
>   			free(buffer_stream->file);
> -			ret = -ENOMEM;
> -			break;
> +			buffer_stream->name = NULL;
> +			buffer_stream->file = NULL;
> +			return -ENOMEM;
>   		}
>   
>   		ret = kshark_tep_stream_init(buffer_stream, buffer_input);

[PATCH v2 24/34] kernelshark: Fix potential memory leaks in libkshark-tepdata

[Benjamin ROBIN]

- In tepdata_get_field_names(), buffer was never free on error
- In kshark_tep_open_buffer(), names were never free if
  kshark_get_data_stream() failed
- In kshark_tep_open_buffer(), prevent any double free error with
  "name" and "file" fields of buffer_stream
- In kshark_tep_init_all_buffers(), return failure code if failed to
  copy "name" and "file" fields of buffer_stream

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/libkshark-tepdata.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/src/libkshark-tepdata.c b/src/libkshark-tepdata.c
index 2d0fcb0..ea647f9 100644
--- a/src/libkshark-tepdata.c
+++ b/src/libkshark-tepdata.c
@@ -949,6 +949,7 @@ static int tepdata_get_field_names(struct kshark_data_stream *stream,
 	for (i = 0; i < nr_fields; ++i)
 		free(buffer[i]);
 
+	free(buffer);
 	return -EFAULT;
 }
 
@@ -1424,8 +1425,10 @@ int kshark_tep_open_buffer(struct kshark_context *kshark_ctx, int sd,
 
 	sd_buffer = kshark_add_stream(kshark_ctx);
 	buffer_stream = kshark_get_data_stream(kshark_ctx, sd_buffer);
-	if (!buffer_stream)
-		return -EFAULT;
+	if (!buffer_stream) {
+		ret = -EFAULT;
+		goto end;
+	}
 
 	for (i = 0; i < n_buffers; ++i) {
 		if (strcmp(buffer_name, names[i]) == 0) {
@@ -1438,7 +1441,8 @@ int kshark_tep_open_buffer(struct kshark_context *kshark_ctx, int sd,
 			if (!buffer_stream->name || !buffer_stream->file) {
 				free(buffer_stream->name);
 				free(buffer_stream->file);
-
+				buffer_stream->name = NULL;
+				buffer_stream->file = NULL;
 				ret = -ENOMEM;
 				break;
 			}
@@ -1449,6 +1453,7 @@ int kshark_tep_open_buffer(struct kshark_context *kshark_ctx, int sd,
 		}
 	}
 
+end:
 	for (i = 0; i < n_buffers; ++i)
 		free(names[i]);
 	free(names);
@@ -1500,8 +1505,9 @@ int kshark_tep_init_all_buffers(struct kshark_context *kshark_ctx,
 		if (!buffer_stream->name || !buffer_stream->file) {
 			free(buffer_stream->name);
 			free(buffer_stream->file);
-			ret = -ENOMEM;
-			break;
+			buffer_stream->name = NULL;
+			buffer_stream->file = NULL;
+			return -ENOMEM;
 		}
 
 		ret = kshark_tep_stream_init(buffer_stream, buffer_input);
-- 
2.43.0

[PATCH 25/34] kernelshark: Fix typo in comment of KsGLWidget::resizeGL()

[Benjamin ROBIN]

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/KsGLWidget.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/KsGLWidget.cpp b/src/KsGLWidget.cpp
index dd9f41a..9311d98 100644
--- a/src/KsGLWidget.cpp
+++ b/src/KsGLWidget.cpp
@@ -93,7 +93,7 @@ void KsGLWidget::initializeGL()
 }
 
 /**
- * Reimplemented function used to reprocess all graphs whene the widget has
+ * Reimplemented function used to reprocess all graphs when the widget has
  * been resized.
  */
 void KsGLWidget::resizeGL(int w, int h)
-- 
2.43.0

[PATCH 26/34] kernelshark: Remove unused KsDataWidget::wipPtr() and broken function

[Benjamin ROBIN]

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/KsWidgetsLib.hpp | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/src/KsWidgetsLib.hpp b/src/KsWidgetsLib.hpp
index e21441d..48108cd 100644
--- a/src/KsWidgetsLib.hpp
+++ b/src/KsWidgetsLib.hpp
@@ -123,12 +123,6 @@ public:
 	explicit KsDataWidget(QWidget *parent = nullptr)
 	: QWidget(parent), _workInProgress(nullptr) {}
 
-	/** Set a pointer to the KsWorkInProgress widget. */
-	const KsWorkInProgress *wipPtr(KsWorkInProgress *wip) const
-	{
-		return _workInProgress;
-	}
-
 	/** Set the pointer to the KsWorkInProgress widget. */
 	void setWipPtr(KsWorkInProgress *wip)
 	{
-- 
2.43.0

[PATCH 27/34] kernelshark: In KsTimeOffsetDialog() constructor use parent param

[Benjamin ROBIN]

Pass parent parameter to QDialog() constructor

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/KsWidgetsLib.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/KsWidgetsLib.cpp b/src/KsWidgetsLib.cpp
index 3ad93fa..5191c9f 100644
--- a/src/KsWidgetsLib.cpp
+++ b/src/KsWidgetsLib.cpp
@@ -209,7 +209,7 @@ bool fileExistsDialog(QString fileName)
 }
 
 /** Create KsTimeOffsetDialog. */
-KsTimeOffsetDialog::KsTimeOffsetDialog(QWidget *parent)
+KsTimeOffsetDialog::KsTimeOffsetDialog(QWidget *parent) : QDialog(parent)
 {
 	kshark_context *kshark_ctx(nullptr);
 	QVector<int> streamIds;
-- 
2.43.0

[PATCH 28/34] kernelshark: Fixed loop counter incremented suspiciously twice

[Benjamin ROBIN]

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 examples/datafilter.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/examples/datafilter.c b/examples/datafilter.c
index 38afab8..8e86d9c 100644
--- a/examples/datafilter.c
+++ b/examples/datafilter.c
@@ -69,7 +69,6 @@ int main(int argc, char **argv)
 
 	/* Print to the screen the first 10 visible entries. */
 	count = 0;
-	i = 0;
 	for (i = 0; i < n_rows; ++i) {
 		if (data[i]->visible & KS_TEXT_VIEW_FILTER_MASK) {
 			entry_str = kshark_dump_entry(data[i]);
@@ -79,8 +78,6 @@ int main(int argc, char **argv)
 			if (++count > 10)
 				break;
 		}
-
-		++i;
 	}
 
 	puts("\n\n");
@@ -102,7 +99,6 @@ int main(int argc, char **argv)
 
 	/* Print to the screen the first 10 visible entries. */
 	count = 0;
-	i = 0;
 	for (i = 0; i < n_rows; ++i) {
 		if (data[i]->visible & KS_TEXT_VIEW_FILTER_MASK) {
 			entry_str = kshark_dump_entry(data[i]);
@@ -112,8 +108,6 @@ int main(int argc, char **argv)
 			if (++count > 10)
 				break;
 		}
-
-		++i;
 	}
 
 	puts("\n\n");
-- 
2.43.0

[PATCH 29/34] kernelshark: Fix tepdata_dump_entry() for event_id = KS_EVENT_OVERFLOW

[Benjamin ROBIN]

Generated string was dropped, and did generate a memory leak,
because of the missing break.

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/libkshark-tepdata.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/libkshark-tepdata.c b/src/libkshark-tepdata.c
index d15c155..a178de6 100644
--- a/src/libkshark-tepdata.c
+++ b/src/libkshark-tepdata.c
@@ -1047,6 +1047,7 @@ static char *tepdata_dump_entry(struct kshark_data_stream *stream,
 		case KS_EVENT_OVERFLOW:
 			entry_str = tepdata_dump_custom_entry(stream, entry,
 							     missed_events_dump);
+			break;
 		default:
 			return NULL;
 		}
-- 
2.43.0

[PATCH 30/34] kernelshark: Use static_cast instead of C cast in KsMainWindow

[Benjamin ROBIN]

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/KsMainWindow.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/KsMainWindow.cpp b/src/KsMainWindow.cpp
index 72ad371..72f4280 100644
--- a/src/KsMainWindow.cpp
+++ b/src/KsMainWindow.cpp
@@ -1546,7 +1546,7 @@ void KsMainWindow::_captureFinished(int ret, QProcess::ExitStatus st)
 
 void KsMainWindow::_captureError(QProcess::ProcessError error)
 {
-	QProcess *capture = (QProcess *)sender();
+	QProcess *capture = static_cast<QProcess*>(sender());
 	_captureErrorMessage(capture);
 }
 
-- 
2.43.0

[PATCH 31/34] kernelshark: Fix comparison of integers of different signs warnings

[Benjamin ROBIN]

Most of them have been fixed, but a few still remain.

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 examples/configio.c        |  3 ++-
 examples/datafilter.c      |  9 ++++++---
 examples/datahisto.c       |  2 +-
 src/libkshark-collection.c | 14 ++++++++------
 src/libkshark-configio.c   |  6 ++++--
 src/libkshark-hash.c       |  5 +++--
 src/libkshark-model.c      | 16 +++++++++-------
 src/libkshark-tepdata.c    |  2 +-
 src/libkshark.c            | 17 +++++++++--------
 src/plugins/sched_events.c |  2 +-
 10 files changed, 44 insertions(+), 32 deletions(-)

diff --git a/examples/configio.c b/examples/configio.c
index 9710d53..575211d 100644
--- a/examples/configio.c
+++ b/examples/configio.c
@@ -8,7 +8,8 @@ int main(int argc, char **argv)
 	struct kshark_config_doc *conf, *filter, *hello;
 	struct kshark_context *kshark_ctx;
 	struct kshark_data_stream *stream;
-	int sd, *ids = NULL, i;
+	int sd, *ids = NULL;
+	size_t i;
 
 	/* Create a new kshark session. */
 	kshark_ctx = NULL;
diff --git a/examples/datafilter.c b/examples/datafilter.c
index 8e86d9c..21a38fe 100644
--- a/examples/datafilter.c
+++ b/examples/datafilter.c
@@ -23,6 +23,7 @@ int main(int argc, char **argv)
 	struct kshark_entry **data = NULL;
 	int *pids, *evt_ids;
 	char *entry_str;
+	int rt;
 
 	/* Create a new kshark session. */
 	kshark_ctx = NULL;
@@ -31,15 +32,17 @@ int main(int argc, char **argv)
 
 	/* Open a trace data file produced by trace-cmd. */
 	if (argc > 1)
-		sd = kshark_open(kshark_ctx, argv[1]);
+		rt = kshark_open(kshark_ctx, argv[1]);
 	else
-		sd = kshark_open(kshark_ctx, default_file);
+		rt = kshark_open(kshark_ctx, default_file);
 
-	if (sd < 0) {
+	if (rt < 0) {
 		kshark_free(kshark_ctx);
 		return 1;
 	}
 
+	sd = (size_t)rt;
+
 	/* Load the content of the file into an array of entries. */
 	n_rows = kshark_load_entries(kshark_ctx, sd, &data);
 
diff --git a/examples/datahisto.c b/examples/datahisto.c
index 568072d..b54b9e9 100644
--- a/examples/datahisto.c
+++ b/examples/datahisto.c
@@ -70,7 +70,7 @@ void dump_bin(struct kshark_trace_histo *histo, int bin, int sd,
 
 void dump_histo(struct kshark_trace_histo *histo, int sd, const char *type, int val)
 {
-	size_t bin;
+	int bin;
 
 	for (bin = 0; bin < histo->n_bins; ++bin)
 		dump_bin(histo, bin, sd, type, val);
diff --git a/src/libkshark-collection.c b/src/libkshark-collection.c
index 915983b..2ce113f 100644
--- a/src/libkshark-collection.c
+++ b/src/libkshark-collection.c
@@ -518,14 +518,16 @@ static int
 map_collection_front_request(const struct kshark_entry_collection *col,
 			     struct kshark_entry_request *req)
 {
-	size_t req_first, req_end;
-	ssize_t col_index;
+	size_t req_first, req_end, col_index;
+	ssize_t r;
 	int req_count;
 
-	col_index = map_collection_request_init(col, req, true, &req_end);
-	if (col_index == KS_EMPTY_BIN)
+	r = map_collection_request_init(col, req, true, &req_end);
+	if (r == KS_EMPTY_BIN)
 		return 0;
 
+	col_index = (size_t)r;
+
 	/*
 	 * Now loop over the intervals of the collection going forwards till
 	 * the end of the inputted request and create a separate request for
@@ -746,7 +748,7 @@ kshark_find_data_collection(struct kshark_entry_collection *col,
 	while (col) {
 		if (col->cond == cond &&
 		    col->stream_id == sd &&
-		    col->n_val == n_val &&
+		    (size_t)col->n_val == n_val &&
 		    val_compare(col->values, values, n_val))
 				return col;
 
@@ -899,7 +901,7 @@ void kshark_unregister_data_collection(struct kshark_entry_collection **col,
 	for (list = *col; list; list = list->next) {
 		if (list->cond == cond &&
 		    list->stream_id == sd &&
-		    list->n_val == n_val &&
+		    (size_t)list->n_val == n_val &&
 		    val_compare(list->values, values, n_val)) {
 			*last = list->next;
 			kshark_free_data_collection(list);
diff --git a/src/libkshark-configio.c b/src/libkshark-configio.c
index 49f957b..0694e19 100644
--- a/src/libkshark-configio.c
+++ b/src/libkshark-configio.c
@@ -1117,7 +1117,8 @@ static bool kshark_event_filter_to_json(struct kshark_data_stream *stream,
 	json_object *jfilter_data, *jname;
 	struct kshark_hash_id *filter;
 	char *name_str;
-	int i, *ids;
+	int *ids;
+	size_t i;
 
 	filter = kshark_get_filter(stream, filter_type);
 	if (!filter)
@@ -1286,7 +1287,8 @@ static bool kshark_filter_array_to_json(struct kshark_hash_id *filter,
 					struct json_object *jobj)
 {
 	json_object *jfilter_data, *jpid = NULL;
-	int i, *ids;
+	int *ids;
+	size_t i;
 
 	/*
 	 * If this Json document already contains a description of the filter,
diff --git a/src/libkshark-hash.c b/src/libkshark-hash.c
index 89c021b..d4f0a31 100644
--- a/src/libkshark-hash.c
+++ b/src/libkshark-hash.c
@@ -169,7 +169,7 @@ void kshark_hash_id_clear(struct kshark_hash_id *hash)
 {
 	struct kshark_hash_id_item *item, *next;
 	size_t size;
-	int i;
+	size_t i;
 
 	if (!hash || ! hash->hash)
 		return;
@@ -212,7 +212,8 @@ int *kshark_hash_ids(struct kshark_hash_id *hash)
 {
 	struct kshark_hash_id_item *item;
 	size_t size = hash_size(hash);
-	int count = 0, i;
+	size_t i;
+	int count = 0;
 	int *ids;
 
 	if (!hash->count)
diff --git a/src/libkshark-model.c b/src/libkshark-model.c
index 4cd9f6a..2918567 100644
--- a/src/libkshark-model.c
+++ b/src/libkshark-model.c
@@ -97,6 +97,8 @@ static void ksmodel_set_in_range_bining(struct kshark_trace_histo *histo,
 	int64_t corrected_range, delta_range, range = max - min;
 	struct kshark_entry *last;
 
+	assert(min <= max);
+
 	if (n <= 0) {
 		histo->n_bins = histo->bin_size = 0;
 		histo->min = min;
@@ -110,7 +112,7 @@ static void ksmodel_set_in_range_bining(struct kshark_trace_histo *histo,
 	}
 
 	/* The size of the bin must be >= 1, hence the range must be >= n. */
-	if (range < n) {
+	if ((size_t)range < n) {
 		range = n;
 		max = min + n;
 	}
@@ -119,7 +121,7 @@ static void ksmodel_set_in_range_bining(struct kshark_trace_histo *histo,
 	 * If the number of bins changes, allocate memory for the descriptor of
 	 * the model.
 	 */
-	if (n != histo->n_bins) {
+	if (n != (size_t)histo->n_bins) {
 		if (!ksmodel_histo_alloc(histo, n)) {
 			ksmodel_clear(histo);
 			return;
@@ -282,7 +284,7 @@ static void ksmodel_set_next_bin_edge(struct kshark_trace_histo *histo,
 	 * case we have to increase the size of the very last bin in order to
 	 * make sure that the last entry of the dataset will fall into it.
 	 */
-	if (next_bin == histo->n_bins - 1)
+	if (next_bin == (size_t)(histo->n_bins - 1))
 		++time_max;
 
 	/*
@@ -544,7 +546,7 @@ void ksmodel_shift_forward(struct kshark_trace_histo *histo, size_t n)
 void ksmodel_shift_backward(struct kshark_trace_histo *histo, size_t n)
 {
 	size_t last_row = 0;
-	int bin;
+	size_t bin;
 
 	if (!histo->data_size)
 		return;
@@ -561,7 +563,7 @@ void ksmodel_shift_backward(struct kshark_trace_histo *histo, size_t n)
 	histo->min -= n * histo->bin_size;
 	histo->max -= n * histo->bin_size;
 
-	if (n >= histo->n_bins) {
+	if (n >= (size_t)histo->n_bins) {
 		/*
 		 * No overlap between the new and the old range. Recalculate
 		 * all bins from scratch. First calculate the new range.
@@ -649,7 +651,7 @@ void ksmodel_jump_to(struct kshark_trace_histo *histo, int64_t ts)
 static void ksmodel_zoom(struct kshark_trace_histo *histo,
 			 double r, int mark, bool zoom_in)
 {
-	size_t range, min, max, delta_min;
+	int64_t range, min, max, delta_min;
 	double delta_tot;
 
 	if (!histo->data_size)
@@ -668,7 +670,7 @@ static void ksmodel_zoom(struct kshark_trace_histo *histo,
 	 * Avoid overzooming. If needed, adjust the Scale factor to a the value
 	 * which provides bin_size >= 5.
 	 */
-	if (zoom_in && (size_t) (range * (1. - r)) < histo->n_bins * 5)
+	if (zoom_in && (int)(range * (1. - r)) < (histo->n_bins * 5))
 		r = 1. - (histo->n_bins * 5.) / range;
 
 	/*
diff --git a/src/libkshark-tepdata.c b/src/libkshark-tepdata.c
index a178de6..4171fab 100644
--- a/src/libkshark-tepdata.c
+++ b/src/libkshark-tepdata.c
@@ -239,7 +239,7 @@ static int get_next_pid(struct kshark_data_stream *stream,
 	ret = tep_read_number_field(get_sched_next(stream),
 				    record->data, &val);
 
-	return ret ? : val;
+	return ret ? : (int)val;
 }
 
 static void register_command(struct kshark_data_stream *stream,
diff --git a/src/libkshark.c b/src/libkshark.c
index 44e553f..698b8d6 100644
--- a/src/libkshark.c
+++ b/src/libkshark.c
@@ -1371,9 +1371,10 @@ void kshark_clear_all_filters(struct kshark_context *kshark_ctx,
 {
 	struct kshark_data_stream *stream;
 	int *stream_ids, i;
+	size_t j;
 
-	for (i = 0; i < n_entries; ++i)
-		set_all_visible(&data[i]->visible);
+	for (j = 0; j < n_entries; ++j)
+		set_all_visible(&data[j]->visible);
 
 	stream_ids = kshark_all_streams(kshark_ctx);
 	for (i = 0; i < kshark_ctx->n_streams; i++) {
@@ -1942,7 +1943,7 @@ kshark_merge_data_entries(struct kshark_entry_data_set *buffers, int n_buffers)
 		return NULL;
 	}
 
-	for (i = 0; i < n_buffers; ++i) {
+	for (i = 0; i < (size_t)n_buffers; ++i) {
 		count[i] = 0;
 		if (buffers[i].n_rows > 0)
 			tot += buffers[i].n_rows;
@@ -2111,7 +2112,7 @@ kshark_merge_data_matrices(struct kshark_matrix_data_set *buffers, int n_buffers
 {
 	struct kshark_matrix_data_set merged_data;
 	size_t i, tot = 0, count[n_buffers];
-	int i_first;
+	int i_first, j;
 	bool status;
 
 	merged_data.n_rows = -1;
@@ -2121,10 +2122,10 @@ kshark_merge_data_matrices(struct kshark_matrix_data_set *buffers, int n_buffers
 		goto end;
 	}
 
-	for (i = 0; i < n_buffers; ++i) {
-		count[i] = 0;
-		if (buffers[i].n_rows > 0)
-			tot += buffers[i].n_rows;
+	for (j = 0; j < n_buffers; ++j) {
+		count[j] = 0;
+		if (buffers[j].n_rows > 0)
+			tot += buffers[j].n_rows;
 	}
 
 	status = kshark_data_matrix_alloc(tot, &merged_data.event_array,
diff --git a/src/plugins/sched_events.c b/src/plugins/sched_events.c
index c3a4f47..a605fca 100644
--- a/src/plugins/sched_events.c
+++ b/src/plugins/sched_events.c
@@ -115,7 +115,7 @@ static void plugin_sched_swith_action(struct kshark_data_stream *stream,
 	ret = tep_read_number_field(plugin_ctx->sched_switch_next_field,
 				    record->data, &next_pid);
 
-	if (ret == 0 && next_pid >= 0) {
+	if (ret == 0) {
 		plugin_sched_set_pid(&ks_field, entry->pid);
 
 		ret = tep_read_number_field(plugin_ctx->sched_switch_prev_state_field,
-- 
2.43.0

Re: [PATCH 31/34] kernelshark: Fix comparison of integers of different signs warnings

[Yordan Karadzhov]

On 1/14/24 19:17, Benjamin ROBIN wrote:
> Most of them have been fixed, but a few still remain.
> 
> Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
> ---
>   examples/configio.c        |  3 ++-
>   examples/datafilter.c      |  9 ++++++---
>   examples/datahisto.c       |  2 +-
>   src/libkshark-collection.c | 14 ++++++++------
>   src/libkshark-configio.c   |  6 ++++--
>   src/libkshark-hash.c       |  5 +++--
>   src/libkshark-model.c      | 16 +++++++++-------
>   src/libkshark-tepdata.c    |  2 +-
>   src/libkshark.c            | 17 +++++++++--------
>   src/plugins/sched_events.c |  2 +-
>   10 files changed, 44 insertions(+), 32 deletions(-)
> 
> diff --git a/examples/configio.c b/examples/configio.c
> index 9710d53..575211d 100644
> --- a/examples/configio.c
> +++ b/examples/configio.c
> @@ -8,7 +8,8 @@ int main(int argc, char **argv)
>   	struct kshark_config_doc *conf, *filter, *hello;
>   	struct kshark_context *kshark_ctx;
>   	struct kshark_data_stream *stream;
> -	int sd, *ids = NULL, i;
> +	int sd, *ids = NULL;
> +	size_t i;
>   
>   	/* Create a new kshark session. */
>   	kshark_ctx = NULL;
> diff --git a/examples/datafilter.c b/examples/datafilter.c
> index 8e86d9c..21a38fe 100644
> --- a/examples/datafilter.c
> +++ b/examples/datafilter.c
> @@ -23,6 +23,7 @@ int main(int argc, char **argv)
>   	struct kshark_entry **data = NULL;
>   	int *pids, *evt_ids;
>   	char *entry_str;
> +	int rt;

'sd' should be of type int. No need to declare 'rt'.

>   
>   	/* Create a new kshark session. */
>   	kshark_ctx = NULL;
> @@ -31,15 +32,17 @@ int main(int argc, char **argv)
>   
>   	/* Open a trace data file produced by trace-cmd. */
>   	if (argc > 1)
> -		sd = kshark_open(kshark_ctx, argv[1]);
> +		rt = kshark_open(kshark_ctx, argv[1]);
>   	else
> -		sd = kshark_open(kshark_ctx, default_file);
> +		rt = kshark_open(kshark_ctx, default_file);
>   
> -	if (sd < 0) {
> +	if (rt < 0) {
>   		kshark_free(kshark_ctx);
>   		return 1;
>   	}
>   
> +	sd = (size_t)rt;
> +
>   	/* Load the content of the file into an array of entries. */
>   	n_rows = kshark_load_entries(kshark_ctx, sd, &data);
>   
> diff --git a/examples/datahisto.c b/examples/datahisto.c
> index 568072d..b54b9e9 100644
> --- a/examples/datahisto.c
> +++ b/examples/datahisto.c
> @@ -70,7 +70,7 @@ void dump_bin(struct kshark_trace_histo *histo, int bin, int sd,
>   
>   void dump_histo(struct kshark_trace_histo *histo, int sd, const char *type, int val)
>   {
> -	size_t bin;
> +	int bin;
>   
>   	for (bin = 0; bin < histo->n_bins; ++bin)
>   		dump_bin(histo, bin, sd, type, val);
> diff --git a/src/libkshark-collection.c b/src/libkshark-collection.c
> index 915983b..2ce113f 100644
> --- a/src/libkshark-collection.c
> +++ b/src/libkshark-collection.c
> @@ -518,14 +518,16 @@ static int
>   map_collection_front_request(const struct kshark_entry_collection *col,
>   			     struct kshark_entry_request *req)
>   {
> -	size_t req_first, req_end;
> -	ssize_t col_index;
> +	size_t req_first, req_end, col_index;
> +	ssize_t r;

We are trying to use self explanatory variable names  if possible.
In many places in the code for variables like this one we use 'ret'.

>   	int req_count;
>   
> -	col_index = map_collection_request_init(col, req, true, &req_end);
> -	if (col_index == KS_EMPTY_BIN)
> +	r = map_collection_request_init(col, req, true, &req_end);
> +	if (r == KS_EMPTY_BIN)
>   		return 0;
>   
> +	col_index = (size_t)r;
> +
>   	/*
>   	 * Now loop over the intervals of the collection going forwards till
>   	 * the end of the inputted request and create a separate request for
> @@ -746,7 +748,7 @@ kshark_find_data_collection(struct kshark_entry_collection *col,
>   	while (col) {
>   		if (col->cond == cond &&
>   		    col->stream_id == sd &&
> -		    col->n_val == n_val &&
> +		    (size_t)col->n_val == n_val &&
>   		    val_compare(col->values, values, n_val))
>   				return col;
>   
> @@ -899,7 +901,7 @@ void kshark_unregister_data_collection(struct kshark_entry_collection **col,
>   	for (list = *col; list; list = list->next) {
>   		if (list->cond == cond &&
>   		    list->stream_id == sd &&
> -		    list->n_val == n_val &&
> +		    (size_t)list->n_val == n_val &&
>   		    val_compare(list->values, values, n_val)) {
>   			*last = list->next;
>   			kshark_free_data_collection(list);
> diff --git a/src/libkshark-configio.c b/src/libkshark-configio.c
> index 49f957b..0694e19 100644
> --- a/src/libkshark-configio.c
> +++ b/src/libkshark-configio.c
> @@ -1117,7 +1117,8 @@ static bool kshark_event_filter_to_json(struct kshark_data_stream *stream,
>   	json_object *jfilter_data, *jname;
>   	struct kshark_hash_id *filter;
>   	char *name_str;
> -	int i, *ids;
> +	int *ids;
> +	size_t i;
>   
>   	filter = kshark_get_filter(stream, filter_type);
>   	if (!filter)
> @@ -1286,7 +1287,8 @@ static bool kshark_filter_array_to_json(struct kshark_hash_id *filter,
>   					struct json_object *jobj)
>   {
>   	json_object *jfilter_data, *jpid = NULL;
> -	int i, *ids;
> +	int *ids;
> +	size_t i;
>   
>   	/*
>   	 * If this Json document already contains a description of the filter,
> diff --git a/src/libkshark-hash.c b/src/libkshark-hash.c
> index 89c021b..d4f0a31 100644
> --- a/src/libkshark-hash.c
> +++ b/src/libkshark-hash.c
> @@ -169,7 +169,7 @@ void kshark_hash_id_clear(struct kshark_hash_id *hash)
>   {
>   	struct kshark_hash_id_item *item, *next;
>   	size_t size;
> -	int i;
> +	size_t i;
>   
>   	if (!hash || ! hash->hash)
>   		return;
> @@ -212,7 +212,8 @@ int *kshark_hash_ids(struct kshark_hash_id *hash)
>   {
>   	struct kshark_hash_id_item *item;
>   	size_t size = hash_size(hash);
> -	int count = 0, i;
> +	size_t i;
> +	int count = 0;
>   	int *ids;
>   
>   	if (!hash->count)
> diff --git a/src/libkshark-model.c b/src/libkshark-model.c
> index 4cd9f6a..2918567 100644
> --- a/src/libkshark-model.c
> +++ b/src/libkshark-model.c
> @@ -97,6 +97,8 @@ static void ksmodel_set_in_range_bining(struct kshark_trace_histo *histo,
>   	int64_t corrected_range, delta_range, range = max - min;
>   	struct kshark_entry *last;
>   
> +	assert(min <= max);
Having this assert() makes it safe to declare 'range' as size_t.

> +
>   	if (n <= 0) {
>   		histo->n_bins = histo->bin_size = 0;
>   		histo->min = min;
> @@ -110,7 +112,7 @@ static void ksmodel_set_in_range_bining(struct kshark_trace_histo *histo,
>   	}
>   
>   	/* The size of the bin must be >= 1, hence the range must be >= n. */
> -	if (range < n) {
> +	if ((size_t)range < n) {
>   		range = n;
>   		max = min + n;
>   	}
> @@ -119,7 +121,7 @@ static void ksmodel_set_in_range_bining(struct kshark_trace_histo *histo,
>   	 * If the number of bins changes, allocate memory for the descriptor of
>   	 * the model.
>   	 */
> -	if (n != histo->n_bins) {

I think the problem here is that the function argument 'n' is declared
with wrong type. It must match the type of histo->n_bins which is int.

> +	if (n != (size_t)histo->n_bins) {
>   		if (!ksmodel_histo_alloc(histo, n)) {
>   			ksmodel_clear(histo);
>   			return;
> @@ -282,7 +284,7 @@ static void ksmodel_set_next_bin_edge(struct kshark_trace_histo *histo,
>   	 * case we have to increase the size of the very last bin in order to
>   	 * make sure that the last entry of the dataset will fall into it.
>   	 */
> -	if (next_bin == histo->n_bins - 1)
> +	if (next_bin == (size_t)(histo->n_bins - 1))
>   		++time_max;
>   
>   	/*
> @@ -544,7 +546,7 @@ void ksmodel_shift_forward(struct kshark_trace_histo *histo, size_t n)
>   void ksmodel_shift_backward(struct kshark_trace_histo *histo, size_t n)
>   {
>   	size_t last_row = 0;
> -	int bin;
> +	size_t bin;
>   
>   	if (!histo->data_size)
>   		return;
> @@ -561,7 +563,7 @@ void ksmodel_shift_backward(struct kshark_trace_histo *histo, size_t n)
>   	histo->min -= n * histo->bin_size;
>   	histo->max -= n * histo->bin_size;
>   
> -	if (n >= histo->n_bins) {
> +	if (n >= (size_t)histo->n_bins) {
>   		/*
>   		 * No overlap between the new and the old range. Recalculate
>   		 * all bins from scratch. First calculate the new range.
> @@ -649,7 +651,7 @@ void ksmodel_jump_to(struct kshark_trace_histo *histo, int64_t ts)
>   static void ksmodel_zoom(struct kshark_trace_histo *histo,
>   			 double r, int mark, bool zoom_in)
>   {
> -	size_t range, min, max, delta_min;
> +	int64_t range, min, max, delta_min;
>   	double delta_tot;
>   
>   	if (!histo->data_size)
> @@ -668,7 +670,7 @@ static void ksmodel_zoom(struct kshark_trace_histo *histo,
>   	 * Avoid overzooming. If needed, adjust the Scale factor to a the value
>   	 * which provides bin_size >= 5.
>   	 */
> -	if (zoom_in && (size_t) (range * (1. - r)) < histo->n_bins * 5)
> +	if (zoom_in && (int)(range * (1. - r)) < (histo->n_bins * 5))
>   		r = 1. - (histo->n_bins * 5.) / range;
>   
>   	/*
> diff --git a/src/libkshark-tepdata.c b/src/libkshark-tepdata.c
> index a178de6..4171fab 100644
> --- a/src/libkshark-tepdata.c
> +++ b/src/libkshark-tepdata.c
> @@ -239,7 +239,7 @@ static int get_next_pid(struct kshark_data_stream *stream,
>   	ret = tep_read_number_field(get_sched_next(stream),
>   				    record->data, &val);
>   
> -	return ret ? : val;
> +	return ret ? : (int)val;
>   }
>   
>   static void register_command(struct kshark_data_stream *stream,
> diff --git a/src/libkshark.c b/src/libkshark.c
> index 44e553f..698b8d6 100644
> --- a/src/libkshark.c
> +++ b/src/libkshark.c
> @@ -1371,9 +1371,10 @@ void kshark_clear_all_filters(struct kshark_context *kshark_ctx,
>   {
>   	struct kshark_data_stream *stream;
>   	int *stream_ids, i;
> +	size_t j;
>   
> -	for (i = 0; i < n_entries; ++i)
> -		set_all_visible(&data[i]->visible);
> +	for (j = 0; j < n_entries; ++j)
> +		set_all_visible(&data[j]->visible);
>   
>   	stream_ids = kshark_all_streams(kshark_ctx);
>   	for (i = 0; i < kshark_ctx->n_streams; i++) {
> @@ -1942,7 +1943,7 @@ kshark_merge_data_entries(struct kshark_entry_data_set *buffers, int n_buffers)
>   		return NULL;
>   	}
>   
> -	for (i = 0; i < n_buffers; ++i) {
> +	for (i = 0; i < (size_t)n_buffers; ++i) {
>   		count[i] = 0;
>   		if (buffers[i].n_rows > 0)
>   			tot += buffers[i].n_rows;
> @@ -2111,7 +2112,7 @@ kshark_merge_data_matrices(struct kshark_matrix_data_set *buffers, int n_buffers
>   {
>   	struct kshark_matrix_data_set merged_data;
>   	size_t i, tot = 0, count[n_buffers];

Note that 'n_buffers' must be size_t as well. Better fix this.

> -	int i_first;
> +	int i_first, j;
>   	bool status;
>   
>   	merged_data.n_rows = -1;
> @@ -2121,10 +2122,10 @@ kshark_merge_data_matrices(struct kshark_matrix_data_set *buffers, int n_buffers
>   		goto end;
>   	}
>   
> -	for (i = 0; i < n_buffers; ++i) {
> -		count[i] = 0;
> -		if (buffers[i].n_rows > 0)
> -			tot += buffers[i].n_rows;
> +	for (j = 0; j < n_buffers; ++j) {
> +		count[j] = 0;
> +		if (buffers[j].n_rows > 0)
> +			tot += buffers[j].n_rows;
>   	}
>   
>   	status = kshark_data_matrix_alloc(tot, &merged_data.event_array,
> diff --git a/src/plugins/sched_events.c b/src/plugins/sched_events.c
> index c3a4f47..a605fca 100644
> --- a/src/plugins/sched_events.c
> +++ b/src/plugins/sched_events.c
> @@ -115,7 +115,7 @@ static void plugin_sched_swith_action(struct kshark_data_stream *stream,
>   	ret = tep_read_number_field(plugin_ctx->sched_switch_next_field,
>   				    record->data, &next_pid);
>   
> -	if (ret == 0 && next_pid >= 0) {
> +	if (ret == 0) {
>   		plugin_sched_set_pid(&ks_field, entry->pid);
>   
>   		ret = tep_read_number_field(plugin_ctx->sched_switch_prev_state_field,

[PATCH 32/34] kernelshark: Fix KsTableView columns width, and KsTraceViewer size

[Benjamin ROBIN]

- Use QSizePolicy::Expanding for KsTraceViewer object to use full height
- Do not use StyleSheet for KsTableView. When the StyleSheet is updated Qt6
  redraw the whole table but with default column width without querying items
  sizeHint(). So instead use QPalette::Highlight.
- Simplify _resizeToContents() since the columns have a proper width with Qt6

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=218351
Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/KsTraceViewer.cpp | 67 ++++++++++++-------------------------------
 src/KsTraceViewer.hpp |  2 ++
 2 files changed, 20 insertions(+), 49 deletions(-)

diff --git a/src/KsTraceViewer.cpp b/src/KsTraceViewer.cpp
index 86e9185..93535a4 100644
--- a/src/KsTraceViewer.cpp
+++ b/src/KsTraceViewer.cpp
@@ -83,7 +83,7 @@ KsTraceViewer::KsTraceViewer(QWidget *parent)
   _mState(nullptr),
   _data(nullptr)
 {
-	this->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Maximum);
+	this->setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Expanding);
 
 	/* Make a search toolbar. */
 	_toolbar.setOrientation(Qt::Horizontal);
@@ -125,6 +125,7 @@ KsTraceViewer::KsTraceViewer(QWidget *parent)
 	int defaultRowHeight = FONT_HEIGHT * 1.25;
 	auto lamSelectionChanged = [this, defaultRowHeight] (const QItemSelection &selected,
 							     const QItemSelection &deselected) {
+
 		if (deselected.count()) {
 			_view.verticalHeader()->resizeSection(deselected.indexes().first().row(),
 							      defaultRowHeight);
@@ -208,33 +209,13 @@ void KsTraceViewer::loadData(KsDataStore *data)
 /** Connect the QTableView widget and the State machine of the Dual marker. */
 void KsTraceViewer::setMarkerSM(KsDualMarkerSM *m)
 {
-	QString styleSheetA, styleSheetB;
-
 	_mState = m;
 	_model.setMarkerColors(_mState->markerA()._color,
 			       _mState->markerB()._color);
 
-	/*
-	 * Assign a property to State A of the Dual marker state machine. When
-	 * the marker is in State A the background color of the selected row
-	 * will be the same as the color of Marker A.
-	 */
-	styleSheetA = "selection-background-color : " +
-		      _mState->markerA()._color.name() + ";";
-
-	_mState->stateAPtr()->assignProperty(&_view, "styleSheet",
-						     styleSheetA);
-
-	/*
-	 * Assign a property to State B. When the marker is in State B the
-	 * background color of the selected row will be the same as the color
-	 * of Marker B.
-	 */
-	styleSheetB = "selection-background-color : " +
-		      _mState->markerB()._color.name() + ";";
-
-	_mState->stateBPtr()->assignProperty(&_view, "styleSheet",
-						     styleSheetB);
+	_viewPalette = _view.palette();
+	_viewPalette.setColor(QPalette::Highlight, _mState->activeMarker()._color);
+	_view.setPalette(_viewPalette);
 }
 
 /** Reset (empty) the table. */
@@ -502,35 +483,38 @@ void KsTraceViewer::markSwitch()
 	ssize_t row;
 
 	/* The state of the Dual marker has changed. Get the new active marker. */
-	DualMarkerState state = _mState->getState();
+	DualMarkerState actState = _mState->getState();
+	DualMarkerState pasState = !actState;
 
 	/* First deal with the passive marker. */
-	if (_mState->getMarker(!state)._isSet) {
+	KsGraphMark &pasMark = _mState->getMarker(pasState);
+	if (pasMark._isSet) {
 		/*
 		 * The passive marker is set. Use the model to color the row of
 		 * the passive marker.
 		 */
-		_model.selectRow(!state, _mState->getMarker(!state)._pos);
+		_model.selectRow(pasState, pasMark._pos);
 	}
 	else {
 		/*
 		 * The passive marker is not set.
 		 * Make sure that the model colors nothing.
 		 */
-		_model.selectRow(!state, KS_NO_ROW_SELECTED);
+		_model.selectRow(pasState, KS_NO_ROW_SELECTED);
 	}
 
 	/*
 	 * Now deal with the active marker. This has to be done after dealing
 	 *  with the model, because changing the model clears the selection.
 	 */
-	if (_mState->getMarker(state)._isSet) {
+	KsGraphMark &actMark = _mState->getMarker(actState);
+	if (actMark._isSet) {
 		/*
 		 * The active marker is set. Use QTableView to select its row.
 		 * The index in the source model is used to retrieve the value
 		 * of the row number in the proxy model.
 		 */
-		row =_mState->getMarker(state)._pos;
+		row = actMark._pos;
 
 		QModelIndex index =
 			_proxyModel.mapFromSource(_model.index(row, 0));
@@ -552,6 +536,9 @@ void KsTraceViewer::markSwitch()
 		_view.clearSelection();
 	}
 
+	_viewPalette.setColor(QPalette::Highlight, actMark._color);
+	_view.setPalette(_viewPalette);
+
 	row = selectedRow();
 	if (row >= 0) {
 		_setSearchIterator(row);
@@ -604,7 +591,7 @@ void KsTraceViewer::keyReleaseEvent(QKeyEvent *event)
 
 void KsTraceViewer::_resizeToContents()
 {
-	int col, rows, columnSize, markRow = selectedRow();
+	int markRow = selectedRow();
 
 	_view.setVisible(false);
 	_view.resizeColumnsToContents();
@@ -617,24 +604,6 @@ void KsTraceViewer::_resizeToContents()
 	 */
 	if (markRow == KS_NO_ROW_SELECTED)
 		_view.clearSelection();
-
-	/*
-	 * Because of some unknown reason some of the columns doesn't get
-	 * resized properly by the code above. We will resize this
-	 * column by hand.
-	 */
-	col = KsViewModel::TRACE_VIEW_COL_STREAM;
-	columnSize = STRING_WIDTH(_model.header().at(col)) + FONT_WIDTH;
-	_view.setColumnWidth(col, columnSize);
-
-	col = KsViewModel::TRACE_VIEW_COL_CPU;
-	columnSize = STRING_WIDTH(_model.header().at(col)) + FONT_WIDTH * 2;
-	_view.setColumnWidth(col, columnSize);
-
-	col = KsViewModel::TRACE_VIEW_COL_INDEX;
-	rows = _model.rowCount({});
-	columnSize = STRING_WIDTH(QString("%1").arg(rows)) + FONT_WIDTH;
-	_view.setColumnWidth(col, columnSize);
 }
 
 //! @cond Doxygen_Suppress
diff --git a/src/KsTraceViewer.hpp b/src/KsTraceViewer.hpp
index 7cc5751..9815787 100644
--- a/src/KsTraceViewer.hpp
+++ b/src/KsTraceViewer.hpp
@@ -123,6 +123,8 @@ private:
 
 	KsTableView	_view;
 
+	QPalette	_viewPalette;
+
 	KsViewModel		_model;
 
 	KsFilterProxyModel	_proxyModel;
-- 
2.43.0

[PATCH 33/34] kernelshark: Allow to reduce a bit more the graph height

[Benjamin ROBIN]

With this configuration, at least one core on the graph can be seen
---
 src/KsTraceGraph.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/KsTraceGraph.cpp b/src/KsTraceGraph.cpp
index bc910c8..4599a12 100644
--- a/src/KsTraceGraph.cpp
+++ b/src/KsTraceGraph.cpp
@@ -593,8 +593,8 @@ void KsTraceGraph::updateGeom()
 	       _layout.contentsMargins().top() +
 	       _layout.contentsMargins().bottom();
 
-	if (hMin > KS_GRAPH_HEIGHT * 8)
-		hMin = KS_GRAPH_HEIGHT * 8;
+	if (hMin > KS_GRAPH_HEIGHT * 6)
+		hMin = KS_GRAPH_HEIGHT * 6;
 
 	setMinimumHeight(hMin);
 
-- 
2.43.0

Re: [PATCH 33/34] kernelshark: Allow to reduce a bit more the graph height

[Yordan Karadzhov]

On 1/14/24 19:17, Benjamin ROBIN wrote:
> With this configuration, at least one core on the graph can be seen
Please provide a better explanation of the problem you are trying to
solve with this patch. It seems that you decrease the minimum height
of the widget. How is this going to make "at least one core on the
graph can be seen"?

And please make sure you signed all your patches.

Thanks!
Y.

> ---
>   src/KsTraceGraph.cpp | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/KsTraceGraph.cpp b/src/KsTraceGraph.cpp
> index bc910c8..4599a12 100644
> --- a/src/KsTraceGraph.cpp
> +++ b/src/KsTraceGraph.cpp
> @@ -593,8 +593,8 @@ void KsTraceGraph::updateGeom()
>   	       _layout.contentsMargins().top() +
>   	       _layout.contentsMargins().bottom();
>   
> -	if (hMin > KS_GRAPH_HEIGHT * 8)
> -		hMin = KS_GRAPH_HEIGHT * 8;
> +	if (hMin > KS_GRAPH_HEIGHT * 6)
> +		hMin = KS_GRAPH_HEIGHT * 6;
>   
>   	setMinimumHeight(hMin);
>   

[PATCH v2 33/34] kernelshark: Allow to reduce a bit more the graph height

[Benjamin ROBIN]

Reduce minimum height of the graph widget which allows the user (using the
splitter widget) to expand a bit more the trace viewer (KsTraceViewer)
which provide the table view (KsTableView).
This is very useful when a user has a very small screen.
With this configuration, at least one CPU core on the graph (KsTraceGraph)
can be seen, so the graph widget is still perfectly usable.

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/KsTraceGraph.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/KsTraceGraph.cpp b/src/KsTraceGraph.cpp
index bc910c8..4599a12 100644
--- a/src/KsTraceGraph.cpp
+++ b/src/KsTraceGraph.cpp
@@ -593,8 +593,8 @@ void KsTraceGraph::updateGeom()
 	       _layout.contentsMargins().top() +
 	       _layout.contentsMargins().bottom();
 
-	if (hMin > KS_GRAPH_HEIGHT * 8)
-		hMin = KS_GRAPH_HEIGHT * 8;
+	if (hMin > KS_GRAPH_HEIGHT * 6)
+		hMin = KS_GRAPH_HEIGHT * 6;
 
 	setMinimumHeight(hMin);
 
-- 
2.43.0

[PATCH 34/34] kernelshark: Cleanup of KsDualMarker methods

[Benjamin ROBIN]

Remove stateAPtr and stateBPtr methods since no longer needed

Signed-off-by: Benjamin ROBIN <dev@benjarobin.fr>
---
 src/KsDualMarker.hpp | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/src/KsDualMarker.hpp b/src/KsDualMarker.hpp
index 73c3960..e81d721 100644
--- a/src/KsDualMarker.hpp
+++ b/src/KsDualMarker.hpp
@@ -144,22 +144,16 @@ public:
 	/** Get the marker B. */
 	KsGraphMark &markerB() {return _markB;}
 
-	/** Get a pointer to the State A object. */
-	QState *stateAPtr() {return _stateA;}
-
-	/** Get a pointer to the State B object. */
-	QState *stateBPtr() {return _stateB;}
-
 	void updateMarkers(const KsDataStore &data,
 			   KsGLWidget *glw);
 
 	void updateLabels();
 
 	/** Get the index inside the data array marker A points to. */
-	ssize_t markerAPos() {return markerA()._isSet ? markerA()._pos : -1;}
+	ssize_t markerAPos() {return _markA._isSet ? _markA._pos : -1;}
 
 	/** Get the index inside the data array marker B points to. */
-	ssize_t markerBPos() {return markerB()._isSet ? markerB()._pos : -1;}
+	ssize_t markerBPos() {return _markB._isSet ? _markB._pos : -1;}
 
 signals:
 	/**
-- 
2.43.0

Re: [PATCH 00/34] Fix kernelshark issues introduced by the migration to Qt6

[Yordan Karadzhov]

Hi Benjamin,

I am applying most of the patches from your patch-set. I have some minor 
comments about few of the changes that I will make in the individual 
patches.

Once again, thanks a lot for helping us to improve kernelshark!

Cheers,
Yordan

On 1/14/24 19:16, Benjamin ROBIN wrote:
> There were 3 majors issues:
>   - A segfault when loading a trace file (patch 0001)
>   - The trace table height was very small (patch 0032)
>   - The trace table columns width were reducing when clicking
>     Marker A or B (patch 0032)
> 
> Also fix most of the warnings reported by Clang-Tidy and Clazy, and by
> gcc with -Wextra.
> 
> 
> Benjamin ROBIN (34):
>    kernelshark: Fix modelReset() signaling, rename update to updateGeom
>    kernelshark: Add .gitignore
>    kernelshark: Remove function param when not used, whenever possible
>    kernelshark: Do not create a temporary container for looping over QMap
>    kernelshark: Prevent potential detach of QMap container
>    kernelshark: Fix used after free of QByteArray raw data
>    kernelshark: Fix potential memory leak in KsGLWidget
>    kernelshark: Use lambda parameter instead of captured local variable
>    kernelshark: Keep overridden method protected instead of public
>    kernelshark: Use sliced() or first() instead of mid/right/left()
>    kernelshark: Prevent potential divide by zero in Shape::center()
>    kernelshark: Fix potential access to uninitialized variable
>    kernelshark: Remove unused locals variables
>    kernelshark: Fix range-loop-reference Clazy warning
>    kernelshark: Fix moving a temp object prevents copy elision warning
>    kernelshark: Add receiver object to connect() call
>    kernelshark: Return by reference the list of header in KsModels
>    kernelshark: Fix detaching-temporary Clazy warning
>    kernelshark: Fix qfileinfo-exists Clazy warning
>    kernelshark: Fix potential memory leaks in libkshark-configio
>    kernelshark: Fix potential access to uninitialized variable
>    kernelshark: Fix potential double free of histo->map, histo->bin_count
>    kernelshark: Fix 'const' type qualifier on return type has no effect
>    kernelshark: Fix potential memory leaks in libkshark-tepdata
>    kernelshark: Fix typo in comment of KsGLWidget::resizeGL()
>    kernelshark: Remove unused KsDataWidget::wipPtr() and broken function
>    kernelshark: In KsTimeOffsetDialog() constructor use parent param
>    kernelshark: Fixed loop counter incremented suspiciously twice
>    kernelshark: Fix tepdata_dump_entry() for event_id = KS_EVENT_OVERFLOW
>    kernelshark: Use static_cast instead of C cast in KsMainWindow
>    kernelshark: Fix comparison of integers of different signs warnings
>    kernelshark: Fix KsTableView columns width, and KsTraceViewer size
>    kernelshark: Allow to reduce a bit more the graph height
>    kernelshark: Cleanup of KsDualMarker methods
> 
>   .gitignore                     | 15 ++++++
>   examples/configio.c            |  3 +-
>   examples/datafilter.c          | 15 +++---
>   examples/datahisto.c           |  2 +-
>   src/KsAdvFilteringDialog.cpp   | 24 ++++------
>   src/KsAdvFilteringDialog.hpp   |  2 +-
>   src/KsDualMarker.hpp           | 10 +---
>   src/KsGLWidget.cpp             | 48 +++++++++----------
>   src/KsGLWidget.hpp             | 43 ++++++++---------
>   src/KsMainWindow.cpp           |  8 ++--
>   src/KsModels.hpp               | 11 +++--
>   src/KsPlotTools.cpp            | 14 +++---
>   src/KsPlotTools.hpp            |  2 +-
>   src/KsSession.cpp              |  4 +-
>   src/KsTraceGraph.cpp           |  7 ++-
>   src/KsTraceViewer.cpp          | 71 ++++++++--------------------
>   src/KsTraceViewer.hpp          | 11 +++--
>   src/KsUtils.cpp                |  9 ++--
>   src/KsUtils.hpp                |  4 +-
>   src/KsWidgetsLib.cpp           |  2 +-
>   src/KsWidgetsLib.hpp           | 15 ++----
>   src/libkshark-collection.c     | 14 +++---
>   src/libkshark-configio.c       | 84 +++++++++++++++++++---------------
>   src/libkshark-hash.c           |  5 +-
>   src/libkshark-model.c          | 19 ++++----
>   src/libkshark-tepdata.c        | 31 ++++++++-----
>   src/libkshark.c                | 17 +++----
>   src/libkshark.h                | 20 ++++----
>   src/plugins/KVMComboDialog.cpp |  7 +--
>   src/plugins/sched_events.c     |  2 +-
>   tests/test-input.c             |  4 +-
>   tests/test-input_ctrl.c        |  4 +-
>   32 files changed, 257 insertions(+), 270 deletions(-)
>   create mode 100644 .gitignore
> 
> --
> 2.43.0
> 

Re: [PATCH 00/34] Fix kernelshark issues introduced by the migration to Qt6

[Benjamin ROBIN]

On Sun, Jan 21, 2024 at 07:08:52PM +0200, Yordan Karadzhov wrote:
> Hi Benjamin,
> 
> I am applying most of the patches from your patch-set. I have some minor
> comments about few of the changes that I will make in the individual
> patches.
> 
> Once again, thanks a lot for helping us to improve kernelshark!
> 
> Cheers,
> Yordan
>

Hi Yordan,

Do you think it is possible to create a new release since all the major bugs 
were resolved?
Indeed, the KernelShark version 2.3.0 is currently not usable, and this is this 
version that is provided in the Arch repository. The Arch maintainers would 
prefer a new release instead of applying a ton of patches.

Thanks,
Benjamin
 
> On 1/14/24 19:16, Benjamin ROBIN wrote:
> > There were 3 majors issues:
> >   - A segfault when loading a trace file (patch 0001)
> >   - The trace table height was very small (patch 0032)
> >   - The trace table columns width were reducing when clicking
> >     Marker A or B (patch 0032)
> > 
> > Also fix most of the warnings reported by Clang-Tidy and Clazy, and by
> > gcc with -Wextra.
> > 
> > 
> > Benjamin ROBIN (34):
> >    kernelshark: Fix modelReset() signaling, rename update to updateGeom
> >    kernelshark: Add .gitignore
> >    kernelshark: Remove function param when not used, whenever possible
> >    kernelshark: Do not create a temporary container for looping over QMap
> >    kernelshark: Prevent potential detach of QMap container
> >    kernelshark: Fix used after free of QByteArray raw data
> >    kernelshark: Fix potential memory leak in KsGLWidget
> >    kernelshark: Use lambda parameter instead of captured local variable
> >    kernelshark: Keep overridden method protected instead of public
> >    kernelshark: Use sliced() or first() instead of mid/right/left()
> >    kernelshark: Prevent potential divide by zero in Shape::center()
> >    kernelshark: Fix potential access to uninitialized variable
> >    kernelshark: Remove unused locals variables
> >    kernelshark: Fix range-loop-reference Clazy warning
> >    kernelshark: Fix moving a temp object prevents copy elision warning
> >    kernelshark: Add receiver object to connect() call
> >    kernelshark: Return by reference the list of header in KsModels
> >    kernelshark: Fix detaching-temporary Clazy warning
> >    kernelshark: Fix qfileinfo-exists Clazy warning
> >    kernelshark: Fix potential memory leaks in libkshark-configio
> >    kernelshark: Fix potential access to uninitialized variable
> >    kernelshark: Fix potential double free of histo->map, histo->bin_count
> >    kernelshark: Fix 'const' type qualifier on return type has no effect
> >    kernelshark: Fix potential memory leaks in libkshark-tepdata
> >    kernelshark: Fix typo in comment of KsGLWidget::resizeGL()
> >    kernelshark: Remove unused KsDataWidget::wipPtr() and broken function
> >    kernelshark: In KsTimeOffsetDialog() constructor use parent param
> >    kernelshark: Fixed loop counter incremented suspiciously twice
> >    kernelshark: Fix tepdata_dump_entry() for event_id = KS_EVENT_OVERFLOW
> >    kernelshark: Use static_cast instead of C cast in KsMainWindow
> >    kernelshark: Fix comparison of integers of different signs warnings
> >    kernelshark: Fix KsTableView columns width, and KsTraceViewer size
> >    kernelshark: Allow to reduce a bit more the graph height
> >    kernelshark: Cleanup of KsDualMarker methods
> > 
> >   .gitignore                     | 15 ++++++
> >   examples/configio.c            |  3 +-
> >   examples/datafilter.c          | 15 +++---
> >   examples/datahisto.c           |  2 +-
> >   src/KsAdvFilteringDialog.cpp   | 24 ++++------
> >   src/KsAdvFilteringDialog.hpp   |  2 +-
> >   src/KsDualMarker.hpp           | 10 +---
> >   src/KsGLWidget.cpp             | 48 +++++++++----------
> >   src/KsGLWidget.hpp             | 43 ++++++++---------
> >   src/KsMainWindow.cpp           |  8 ++--
> >   src/KsModels.hpp               | 11 +++--
> >   src/KsPlotTools.cpp            | 14 +++---
> >   src/KsPlotTools.hpp            |  2 +-
> >   src/KsSession.cpp              |  4 +-
> >   src/KsTraceGraph.cpp           |  7 ++-
> >   src/KsTraceViewer.cpp          | 71 ++++++++--------------------
> >   src/KsTraceViewer.hpp          | 11 +++--
> >   src/KsUtils.cpp                |  9 ++--
> >   src/KsUtils.hpp                |  4 +-
> >   src/KsWidgetsLib.cpp           |  2 +-
> >   src/KsWidgetsLib.hpp           | 15 ++----
> >   src/libkshark-collection.c     | 14 +++---
> >   src/libkshark-configio.c       | 84 +++++++++++++++++++---------------
> >   src/libkshark-hash.c           |  5 +-
> >   src/libkshark-model.c          | 19 ++++----
> >   src/libkshark-tepdata.c        | 31 ++++++++-----
> >   src/libkshark.c                | 17 +++----
> >   src/libkshark.h                | 20 ++++----
> >   src/plugins/KVMComboDialog.cpp |  7 +--
> >   src/plugins/sched_events.c     |  2 +-
> >   tests/test-input.c             |  4 +-
> >   tests/test-input_ctrl.c        |  4 +-
> >   32 files changed, 257 insertions(+), 270 deletions(-)
> >   create mode 100644 .gitignore
> > 
> > --
> > 2.43.0
> > 

Re: [PATCH 00/34] Fix kernelshark issues introduced by the migration to Qt6

[Yordan Karadzhov]

Hi Benjamin,

We still have one unresolved bug that was reported by Sudip.
I will do my best to get this sorted out and have a new release by the 
end of next week.

Thanks,
Y.

On 3/3/24 11:56, Benjamin ROBIN wrote:
> On Sun, Jan 21, 2024 at 07:08:52PM +0200, Yordan Karadzhov wrote:
>> Hi Benjamin,
>>
>> I am applying most of the patches from your patch-set. I have some minor
>> comments about few of the changes that I will make in the individual
>> patches.
>>
>> Once again, thanks a lot for helping us to improve kernelshark!
>>
>> Cheers,
>> Yordan
>>
> 
> Hi Yordan,
> 
> Do you think it is possible to create a new release since all the major bugs
> were resolved?
> Indeed, the KernelShark version 2.3.0 is currently not usable, and this is this
> version that is provided in the Arch repository. The Arch maintainers would
> prefer a new release instead of applying a ton of patches.
> 
> Thanks,
> Benjamin
>   
>> On 1/14/24 19:16, Benjamin ROBIN wrote:
>>> There were 3 majors issues:
>>>    - A segfault when loading a trace file (patch 0001)
>>>    - The trace table height was very small (patch 0032)
>>>    - The trace table columns width were reducing when clicking
>>>      Marker A or B (patch 0032)
>>>
>>> Also fix most of the warnings reported by Clang-Tidy and Clazy, and by
>>> gcc with -Wextra.
>>>
>>>
>>> Benjamin ROBIN (34):
>>>     kernelshark: Fix modelReset() signaling, rename update to updateGeom
>>>     kernelshark: Add .gitignore
>>>     kernelshark: Remove function param when not used, whenever possible
>>>     kernelshark: Do not create a temporary container for looping over QMap
>>>     kernelshark: Prevent potential detach of QMap container
>>>     kernelshark: Fix used after free of QByteArray raw data
>>>     kernelshark: Fix potential memory leak in KsGLWidget
>>>     kernelshark: Use lambda parameter instead of captured local variable
>>>     kernelshark: Keep overridden method protected instead of public
>>>     kernelshark: Use sliced() or first() instead of mid/right/left()
>>>     kernelshark: Prevent potential divide by zero in Shape::center()
>>>     kernelshark: Fix potential access to uninitialized variable
>>>     kernelshark: Remove unused locals variables
>>>     kernelshark: Fix range-loop-reference Clazy warning
>>>     kernelshark: Fix moving a temp object prevents copy elision warning
>>>     kernelshark: Add receiver object to connect() call
>>>     kernelshark: Return by reference the list of header in KsModels
>>>     kernelshark: Fix detaching-temporary Clazy warning
>>>     kernelshark: Fix qfileinfo-exists Clazy warning
>>>     kernelshark: Fix potential memory leaks in libkshark-configio
>>>     kernelshark: Fix potential access to uninitialized variable
>>>     kernelshark: Fix potential double free of histo->map, histo->bin_count
>>>     kernelshark: Fix 'const' type qualifier on return type has no effect
>>>     kernelshark: Fix potential memory leaks in libkshark-tepdata
>>>     kernelshark: Fix typo in comment of KsGLWidget::resizeGL()
>>>     kernelshark: Remove unused KsDataWidget::wipPtr() and broken function
>>>     kernelshark: In KsTimeOffsetDialog() constructor use parent param
>>>     kernelshark: Fixed loop counter incremented suspiciously twice
>>>     kernelshark: Fix tepdata_dump_entry() for event_id = KS_EVENT_OVERFLOW
>>>     kernelshark: Use static_cast instead of C cast in KsMainWindow
>>>     kernelshark: Fix comparison of integers of different signs warnings
>>>     kernelshark: Fix KsTableView columns width, and KsTraceViewer size
>>>     kernelshark: Allow to reduce a bit more the graph height
>>>     kernelshark: Cleanup of KsDualMarker methods
>>>
>>>    .gitignore                     | 15 ++++++
>>>    examples/configio.c            |  3 +-
>>>    examples/datafilter.c          | 15 +++---
>>>    examples/datahisto.c           |  2 +-
>>>    src/KsAdvFilteringDialog.cpp   | 24 ++++------
>>>    src/KsAdvFilteringDialog.hpp   |  2 +-
>>>    src/KsDualMarker.hpp           | 10 +---
>>>    src/KsGLWidget.cpp             | 48 +++++++++----------
>>>    src/KsGLWidget.hpp             | 43 ++++++++---------
>>>    src/KsMainWindow.cpp           |  8 ++--
>>>    src/KsModels.hpp               | 11 +++--
>>>    src/KsPlotTools.cpp            | 14 +++---
>>>    src/KsPlotTools.hpp            |  2 +-
>>>    src/KsSession.cpp              |  4 +-
>>>    src/KsTraceGraph.cpp           |  7 ++-
>>>    src/KsTraceViewer.cpp          | 71 ++++++++--------------------
>>>    src/KsTraceViewer.hpp          | 11 +++--
>>>    src/KsUtils.cpp                |  9 ++--
>>>    src/KsUtils.hpp                |  4 +-
>>>    src/KsWidgetsLib.cpp           |  2 +-
>>>    src/KsWidgetsLib.hpp           | 15 ++----
>>>    src/libkshark-collection.c     | 14 +++---
>>>    src/libkshark-configio.c       | 84 +++++++++++++++++++---------------
>>>    src/libkshark-hash.c           |  5 +-
>>>    src/libkshark-model.c          | 19 ++++----
>>>    src/libkshark-tepdata.c        | 31 ++++++++-----
>>>    src/libkshark.c                | 17 +++----
>>>    src/libkshark.h                | 20 ++++----
>>>    src/plugins/KVMComboDialog.cpp |  7 +--
>>>    src/plugins/sched_events.c     |  2 +-
>>>    tests/test-input.c             |  4 +-
>>>    tests/test-input_ctrl.c        |  4 +-
>>>    32 files changed, 257 insertions(+), 270 deletions(-)
>>>    create mode 100644 .gitignore
>>>
>>> --
>>> 2.43.0
>>>

Re: [PATCH 00/34] Fix kernelshark issues introduced by the migration to Qt6

[Sudip Mukherjee]

Looking at these patches I think my issue should be fixed now. Let me
try the latest HEAD tonight.


-- 
Regards
Sudip


On Sun, 3 Mar 2024 at 15:47, Yordan Karadzhov <y.karadz@gmail.com> wrote:
>
> Hi Benjamin,
>
> We still have one unresolved bug that was reported by Sudip.
> I will do my best to get this sorted out and have a new release by the
> end of next week.
>
> Thanks,
> Y.
>
> On 3/3/24 11:56, Benjamin ROBIN wrote:
> > On Sun, Jan 21, 2024 at 07:08:52PM +0200, Yordan Karadzhov wrote:
> >> Hi Benjamin,
> >>
> >> I am applying most of the patches from your patch-set. I have some minor
> >> comments about few of the changes that I will make in the individual
> >> patches.
> >>
> >> Once again, thanks a lot for helping us to improve kernelshark!
> >>
> >> Cheers,
> >> Yordan
> >>
> >
> > Hi Yordan,
> >
> > Do you think it is possible to create a new release since all the major bugs
> > were resolved?
> > Indeed, the KernelShark version 2.3.0 is currently not usable, and this is this
> > version that is provided in the Arch repository. The Arch maintainers would
> > prefer a new release instead of applying a ton of patches.
> >
> > Thanks,
> > Benjamin
> >
> >> On 1/14/24 19:16, Benjamin ROBIN wrote:
> >>> There were 3 majors issues:
> >>>    - A segfault when loading a trace file (patch 0001)
> >>>    - The trace table height was very small (patch 0032)
> >>>    - The trace table columns width were reducing when clicking
> >>>      Marker A or B (patch 0032)
> >>>
> >>> Also fix most of the warnings reported by Clang-Tidy and Clazy, and by
> >>> gcc with -Wextra.
> >>>
> >>>
> >>> Benjamin ROBIN (34):
> >>>     kernelshark: Fix modelReset() signaling, rename update to updateGeom
> >>>     kernelshark: Add .gitignore
> >>>     kernelshark: Remove function param when not used, whenever possible
> >>>     kernelshark: Do not create a temporary container for looping over QMap
> >>>     kernelshark: Prevent potential detach of QMap container
> >>>     kernelshark: Fix used after free of QByteArray raw data
> >>>     kernelshark: Fix potential memory leak in KsGLWidget
> >>>     kernelshark: Use lambda parameter instead of captured local variable
> >>>     kernelshark: Keep overridden method protected instead of public
> >>>     kernelshark: Use sliced() or first() instead of mid/right/left()
> >>>     kernelshark: Prevent potential divide by zero in Shape::center()
> >>>     kernelshark: Fix potential access to uninitialized variable
> >>>     kernelshark: Remove unused locals variables
> >>>     kernelshark: Fix range-loop-reference Clazy warning
> >>>     kernelshark: Fix moving a temp object prevents copy elision warning
> >>>     kernelshark: Add receiver object to connect() call
> >>>     kernelshark: Return by reference the list of header in KsModels
> >>>     kernelshark: Fix detaching-temporary Clazy warning
> >>>     kernelshark: Fix qfileinfo-exists Clazy warning
> >>>     kernelshark: Fix potential memory leaks in libkshark-configio
> >>>     kernelshark: Fix potential access to uninitialized variable
> >>>     kernelshark: Fix potential double free of histo->map, histo->bin_count
> >>>     kernelshark: Fix 'const' type qualifier on return type has no effect
> >>>     kernelshark: Fix potential memory leaks in libkshark-tepdata
> >>>     kernelshark: Fix typo in comment of KsGLWidget::resizeGL()
> >>>     kernelshark: Remove unused KsDataWidget::wipPtr() and broken function
> >>>     kernelshark: In KsTimeOffsetDialog() constructor use parent param
> >>>     kernelshark: Fixed loop counter incremented suspiciously twice
> >>>     kernelshark: Fix tepdata_dump_entry() for event_id = KS_EVENT_OVERFLOW
> >>>     kernelshark: Use static_cast instead of C cast in KsMainWindow
> >>>     kernelshark: Fix comparison of integers of different signs warnings
> >>>     kernelshark: Fix KsTableView columns width, and KsTraceViewer size
> >>>     kernelshark: Allow to reduce a bit more the graph height
> >>>     kernelshark: Cleanup of KsDualMarker methods
> >>>
> >>>    .gitignore                     | 15 ++++++
> >>>    examples/configio.c            |  3 +-
> >>>    examples/datafilter.c          | 15 +++---
> >>>    examples/datahisto.c           |  2 +-
> >>>    src/KsAdvFilteringDialog.cpp   | 24 ++++------
> >>>    src/KsAdvFilteringDialog.hpp   |  2 +-
> >>>    src/KsDualMarker.hpp           | 10 +---
> >>>    src/KsGLWidget.cpp             | 48 +++++++++----------
> >>>    src/KsGLWidget.hpp             | 43 ++++++++---------
> >>>    src/KsMainWindow.cpp           |  8 ++--
> >>>    src/KsModels.hpp               | 11 +++--
> >>>    src/KsPlotTools.cpp            | 14 +++---
> >>>    src/KsPlotTools.hpp            |  2 +-
> >>>    src/KsSession.cpp              |  4 +-
> >>>    src/KsTraceGraph.cpp           |  7 ++-
> >>>    src/KsTraceViewer.cpp          | 71 ++++++++--------------------
> >>>    src/KsTraceViewer.hpp          | 11 +++--
> >>>    src/KsUtils.cpp                |  9 ++--
> >>>    src/KsUtils.hpp                |  4 +-
> >>>    src/KsWidgetsLib.cpp           |  2 +-
> >>>    src/KsWidgetsLib.hpp           | 15 ++----
> >>>    src/libkshark-collection.c     | 14 +++---
> >>>    src/libkshark-configio.c       | 84 +++++++++++++++++++---------------
> >>>    src/libkshark-hash.c           |  5 +-
> >>>    src/libkshark-model.c          | 19 ++++----
> >>>    src/libkshark-tepdata.c        | 31 ++++++++-----
> >>>    src/libkshark.c                | 17 +++----
> >>>    src/libkshark.h                | 20 ++++----
> >>>    src/plugins/KVMComboDialog.cpp |  7 +--
> >>>    src/plugins/sched_events.c     |  2 +-
> >>>    tests/test-input.c             |  4 +-
> >>>    tests/test-input_ctrl.c        |  4 +-
> >>>    32 files changed, 257 insertions(+), 270 deletions(-)
> >>>    create mode 100644 .gitignore
> >>>
> >>> --
> >>> 2.43.0
> >>>

Re: [PATCH 00/34] Fix kernelshark issues introduced by the migration to Qt6

[Sudip Mukherjee]

I have now tested and can confirm that the patches from Benjamin has
fixed the problem I was seeing on Debian also. A new release will be
better for me also, instead of adding all those patches to Debian
package.

Thanks Benjamin for the fixes.

-- 
Regards
Sudip

On Sun, 3 Mar 2024 at 17:07, Sudip Mukherjee <sudipm.mukherjee@gmail.com> wrote:
>
> Looking at these patches I think my issue should be fixed now. Let me
> try the latest HEAD tonight.
>
>
> --
> Regards
> Sudip
>
>
> On Sun, 3 Mar 2024 at 15:47, Yordan Karadzhov <y.karadz@gmail.com> wrote:
> >
> > Hi Benjamin,
> >
> > We still have one unresolved bug that was reported by Sudip.
> > I will do my best to get this sorted out and have a new release by the
> > end of next week.
> >
> > Thanks,
> > Y.
> >
> > On 3/3/24 11:56, Benjamin ROBIN wrote:
> > > On Sun, Jan 21, 2024 at 07:08:52PM +0200, Yordan Karadzhov wrote:
> > >> Hi Benjamin,
> > >>
> > >> I am applying most of the patches from your patch-set. I have some minor
> > >> comments about few of the changes that I will make in the individual
> > >> patches.
> > >>
> > >> Once again, thanks a lot for helping us to improve kernelshark!
> > >>
> > >> Cheers,
> > >> Yordan
> > >>
> > >
> > > Hi Yordan,
> > >
> > > Do you think it is possible to create a new release since all the major bugs
> > > were resolved?
> > > Indeed, the KernelShark version 2.3.0 is currently not usable, and this is this
> > > version that is provided in the Arch repository. The Arch maintainers would
> > > prefer a new release instead of applying a ton of patches.
> > >
> > > Thanks,
> > > Benjamin
> > >
> > >> On 1/14/24 19:16, Benjamin ROBIN wrote:
> > >>> There were 3 majors issues:
> > >>>    - A segfault when loading a trace file (patch 0001)
> > >>>    - The trace table height was very small (patch 0032)
> > >>>    - The trace table columns width were reducing when clicking
> > >>>      Marker A or B (patch 0032)
> > >>>
> > >>> Also fix most of the warnings reported by Clang-Tidy and Clazy, and by
> > >>> gcc with -Wextra.
> > >>>
> > >>>
> > >>> Benjamin ROBIN (34):
> > >>>     kernelshark: Fix modelReset() signaling, rename update to updateGeom
> > >>>     kernelshark: Add .gitignore
> > >>>     kernelshark: Remove function param when not used, whenever possible
> > >>>     kernelshark: Do not create a temporary container for looping over QMap
> > >>>     kernelshark: Prevent potential detach of QMap container
> > >>>     kernelshark: Fix used after free of QByteArray raw data
> > >>>     kernelshark: Fix potential memory leak in KsGLWidget
> > >>>     kernelshark: Use lambda parameter instead of captured local variable
> > >>>     kernelshark: Keep overridden method protected instead of public
> > >>>     kernelshark: Use sliced() or first() instead of mid/right/left()
> > >>>     kernelshark: Prevent potential divide by zero in Shape::center()
> > >>>     kernelshark: Fix potential access to uninitialized variable
> > >>>     kernelshark: Remove unused locals variables
> > >>>     kernelshark: Fix range-loop-reference Clazy warning
> > >>>     kernelshark: Fix moving a temp object prevents copy elision warning
> > >>>     kernelshark: Add receiver object to connect() call
> > >>>     kernelshark: Return by reference the list of header in KsModels
> > >>>     kernelshark: Fix detaching-temporary Clazy warning
> > >>>     kernelshark: Fix qfileinfo-exists Clazy warning
> > >>>     kernelshark: Fix potential memory leaks in libkshark-configio
> > >>>     kernelshark: Fix potential access to uninitialized variable
> > >>>     kernelshark: Fix potential double free of histo->map, histo->bin_count
> > >>>     kernelshark: Fix 'const' type qualifier on return type has no effect
> > >>>     kernelshark: Fix potential memory leaks in libkshark-tepdata
> > >>>     kernelshark: Fix typo in comment of KsGLWidget::resizeGL()
> > >>>     kernelshark: Remove unused KsDataWidget::wipPtr() and broken function
> > >>>     kernelshark: In KsTimeOffsetDialog() constructor use parent param
> > >>>     kernelshark: Fixed loop counter incremented suspiciously twice
> > >>>     kernelshark: Fix tepdata_dump_entry() for event_id = KS_EVENT_OVERFLOW
> > >>>     kernelshark: Use static_cast instead of C cast in KsMainWindow
> > >>>     kernelshark: Fix comparison of integers of different signs warnings
> > >>>     kernelshark: Fix KsTableView columns width, and KsTraceViewer size
> > >>>     kernelshark: Allow to reduce a bit more the graph height
> > >>>     kernelshark: Cleanup of KsDualMarker methods
> > >>>
> > >>>    .gitignore                     | 15 ++++++
> > >>>    examples/configio.c            |  3 +-
> > >>>    examples/datafilter.c          | 15 +++---
> > >>>    examples/datahisto.c           |  2 +-
> > >>>    src/KsAdvFilteringDialog.cpp   | 24 ++++------
> > >>>    src/KsAdvFilteringDialog.hpp   |  2 +-
> > >>>    src/KsDualMarker.hpp           | 10 +---
> > >>>    src/KsGLWidget.cpp             | 48 +++++++++----------
> > >>>    src/KsGLWidget.hpp             | 43 ++++++++---------
> > >>>    src/KsMainWindow.cpp           |  8 ++--
> > >>>    src/KsModels.hpp               | 11 +++--
> > >>>    src/KsPlotTools.cpp            | 14 +++---
> > >>>    src/KsPlotTools.hpp            |  2 +-
> > >>>    src/KsSession.cpp              |  4 +-
> > >>>    src/KsTraceGraph.cpp           |  7 ++-
> > >>>    src/KsTraceViewer.cpp          | 71 ++++++++--------------------
> > >>>    src/KsTraceViewer.hpp          | 11 +++--
> > >>>    src/KsUtils.cpp                |  9 ++--
> > >>>    src/KsUtils.hpp                |  4 +-
> > >>>    src/KsWidgetsLib.cpp           |  2 +-
> > >>>    src/KsWidgetsLib.hpp           | 15 ++----
> > >>>    src/libkshark-collection.c     | 14 +++---
> > >>>    src/libkshark-configio.c       | 84 +++++++++++++++++++---------------
> > >>>    src/libkshark-hash.c           |  5 +-
> > >>>    src/libkshark-model.c          | 19 ++++----
> > >>>    src/libkshark-tepdata.c        | 31 ++++++++-----
> > >>>    src/libkshark.c                | 17 +++----
> > >>>    src/libkshark.h                | 20 ++++----
> > >>>    src/plugins/KVMComboDialog.cpp |  7 +--
> > >>>    src/plugins/sched_events.c     |  2 +-
> > >>>    tests/test-input.c             |  4 +-
> > >>>    tests/test-input_ctrl.c        |  4 +-
> > >>>    32 files changed, 257 insertions(+), 270 deletions(-)
> > >>>    create mode 100644 .gitignore
> > >>>
> > >>> --
> > >>> 2.43.0
> > >>>