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).
This commit is contained in:
Shane Ringrose
2026-06-20 20:30:35 +12:00
parent f301196f61
commit c586a2d3a3
3 changed files with 15 additions and 6 deletions
+5 -5
View File
@@ -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
{
+5
View File
@@ -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
+5 -1
View File
@@ -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