From c586a2d3a3d468c4f025aec54552442998945b00 Mon Sep 17 00:00:00 2001
From: Shane Ringrose <shane.ringrose@gmail.com>
Date: Sat, 20 Jun 2026 20:30:35 +1200
Subject: [PATCH] Fix three uninitialised-value bugs found by Valgrind

1. machine_info.h: zero-initialise Screen struct members
   Max_width, Max_height, count, width[] and height[] were bare
   int32_t with no initialiser. The comparisons in
   init_get_Screen_info() read them before any write, producing
   undefined behaviour flagged by Valgrind as 'Conditional jump
   or move depends on uninitialised value(s)'.

2. main.cpp: pre-initialise MachineInfo on the main thread
   MachineInfo::instance() was first called inside QtConcurrent::run(),
   causing its constructor (which calls qApp->screens()) to run on a
   background thread. QScreen methods are not thread-safe in Qt5.
   Calling instance() once on the main thread before the worker
   launches guarantees the singleton is fully built first; subsequent
   calls from the worker just return the cached pointer.

3. qetdiagrameditor.h: move m_first_show before the QActionGroup members
   C++ initialises members in declaration order. m_first_show was
   declared after the QActionGroup members (line 256 vs 168). During
   construction of m_row_column_actions_group(this), Qt dispatches a
   QObject parent-change event that reaches QETDiagramEditor::event(),
   which reads m_first_show before it has been initialised.
   Moving the declaration to the top of the first private: block
   ensures it is initialised before any member that can trigger events.

All three found via Valgrind --tool=memcheck on Ubuntu 22.04 / Qt 5.15.3.
Relates-to: PR #514 (same QtConcurrent thread-safety pattern).
---
 sources/machine_info.h     | 10 +++++-----
 sources/main.cpp           |  5 +++++
 sources/qetdiagrameditor.h |  6 +++++-
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/sources/machine_info.h b/sources/machine_info.h
index 04f074f6d..58866184b 100644
--- a/sources/machine_info.h
+++ b/sources/machine_info.h
@@ -75,11 +75,11 @@ class MachineInfo
 	{
 		struct Screen
 		{
-			int32_t count;
-			int32_t width[10];
-			int32_t height[10];
-			int32_t Max_width;
-			int32_t Max_height;
+			int32_t count     = 0;
+			int32_t width[10] = {};
+			int32_t height[10]= {};
+			int32_t Max_width = 0;
+			int32_t Max_height= 0;
 		}screen;
 		struct Built
 		{
diff --git a/sources/main.cpp b/sources/main.cpp
index 8f54279b5..ee70a90ac 100644
--- a/sources/main.cpp
+++ b/sources/main.cpp
@@ -241,6 +241,11 @@ QGuiApplication::setHighDpiScaleFactorRoundingPolicy(QetSettings::hdpiScaleFacto
 	QObject::connect(&app, &SingleApplication::receivedMessage,
 			 &qetapp, &QETApp::receiveMessage);
 
+	// Pre-initialise on the main (GUI) thread: the constructor calls
+	// qApp->screens() which is not thread-safe in Qt5 — calling instance()
+	// here guarantees the singleton is fully built before the worker runs.
+	MachineInfo::instance();
+
 	QtConcurrent::run([=]()
 	{
 		// for debugging
diff --git a/sources/qetdiagrameditor.h b/sources/qetdiagrameditor.h
index d5139ed5a..5a26b5f06 100644
--- a/sources/qetdiagrameditor.h
+++ b/sources/qetdiagrameditor.h
@@ -80,6 +80,11 @@ class QETDiagramEditor : public QETMainWindow
 	  protected:
 		bool event(QEvent *) override;
 	private:
+		// Declared first so it is initialised before any member whose
+		// constructor may dispatch a Qt event calling event() (e.g. the
+		// QActionGroup members below trigger QObject::setParent events).
+		bool m_first_show = true;
+
 		QETDiagramEditor(const QETDiagramEditor &);
 		void setUpElementsPanel ();
 		void setUpElementsCollectionWidget();
@@ -253,7 +258,6 @@ class QETDiagramEditor : public QETMainWindow
 		QUndoGroup undo_group;
 		AutoNumberingDockWidget *m_autonumbering_dock;
 		int activeSubWindowIndex;
-		bool m_first_show = true;
 		SearchAndReplaceWidget m_search_and_replace_widget;
 };
 #endif