Skip to content

Commit 7489e05

Browse files
refactor: smart pointers and fix ownership
Replace raw pointer members (app, trayIcon, trayTopMenu) with std::unique_ptr in QtTrayMenu. Consolidate tray_qt module-level globals into a State struct accessed via a function-local static. Move tray test fixtures from global arrays into per-fixture instance members to eliminate shared mutable state between tests. Replace std::system shell invocations with posix_spawnp for notification cleanup. Use placement new instead of reinterpret_cast+memcpy for C struct initialization over raw buffers.
1 parent 87d324c commit 7489e05

8 files changed

Lines changed: 215 additions & 211 deletions

File tree

.github/workflows/_common-lint.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ concurrency:
1616
jobs:
1717
lint:
1818
name: Common Lint
19-
uses: LizardByte/.github/.github/workflows/__call-common-lint.yml@master
19+
uses: LizardByte/.github/.github/workflows/__call-common-lint.yml@fix/common-lint/clang-format-version
2020
if: ${{ github.repository != 'LizardByte/.github' }}
2121
permissions:
2222
contents: read

src/QtTrayMenu.cpp

Lines changed: 17 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,6 @@
1919
#include "WindowsAppearance.h"
2020
#endif
2121

22-
namespace {
23-
int defaultArgc = 1; // NOSONAR(cpp:S5421): This is required for QApplication's argc/argv constructor
24-
char defaultArgv0[] = "TrayMenuApp"; // NOSONAR(cpp:S5421): This is required for QApplication's argc/argv constructor
25-
char *defaultArgv[] = {defaultArgv0, nullptr}; // NOSONAR(cpp:S5421,cpp:S5954): This is required for QApplication's argc/argv constructor
26-
} // namespace
27-
2822
QtTrayMenu::QtTrayMenu(QObject *parent, const bool debug):
2923
QtTrayMenu(-1, nullptr, parent, debug) {
3024
};
@@ -40,10 +34,11 @@ QtTrayMenu::QtTrayMenu(int argc, char **argv, QObject *parent, const bool debug)
4034
// Note: The following is ugly but QApplication requires an argv containing the application name.
4135
// We might not have access to the real argc/argv here due to being called/pulled as a dependency.
4236
if (argc < 0 && argv == nullptr) {
43-
app = new QApplication(defaultArgc, defaultArgv); // NOSONAR(cpp:S5025): Qt has its own integrated memory management
37+
ownedApp = std::make_unique<QApplication>(defaultArgc, defaultArgv.data());
4438
} else {
45-
app = new QApplication(argc, argv); // NOSONAR(cpp:S5025): Qt has its own integrated memory management
39+
ownedApp = std::make_unique<QApplication>(argc, argv);
4640
}
41+
app = ownedApp.get();
4742
}
4843
#if defined(_WIN32)
4944
tray_qt::windows::configure_appearance(app);
@@ -54,13 +49,8 @@ QtTrayMenu::QtTrayMenu(int argc, char **argv, QObject *parent, const bool debug)
5449
}
5550

5651
QtTrayMenu::~QtTrayMenu() {
57-
// Cleanup app only if it was created within this class
58-
if (app && app != QApplication::instance()) {
59-
// Quit QApplication
52+
if (ownedApp) {
6053
QApplication::quit();
61-
// Delete app and clear references
62-
delete app; // NOSONAR(cpp:S5025): Qt has its own integrated memory management
63-
app = nullptr; // Set to nullptr after deletion
6454
}
6555
}
6656

@@ -82,18 +72,18 @@ int QtTrayMenu::init(struct tray *tray, const bool notification) {
8272
}
8373

8474
// Create tray icon
85-
trayIcon = new QSystemTrayIcon(lookupIcon(tray->icon), this);
75+
trayIcon = std::make_unique<QSystemTrayIcon>(lookupIcon(tray->icon));
8676
trayIcon->setToolTip(QString::fromUtf8(tray->tooltip));
8777

88-
connect(trayIcon, &QSystemTrayIcon::activated, this, &QtTrayMenu::onTrayActivated);
89-
connect(trayIcon, &QSystemTrayIcon::messageClicked, this, &QtTrayMenu::onMessageClicked);
78+
connect(trayIcon.get(), &QSystemTrayIcon::activated, this, &QtTrayMenu::onTrayActivated);
79+
connect(trayIcon.get(), &QSystemTrayIcon::messageClicked, this, &QtTrayMenu::onMessageClicked);
9080
connect(this, &QtTrayMenu::update, this, &QtTrayMenu::onUpdate);
9181
connect(this, &QtTrayMenu::exit, this, &QtTrayMenu::onExitRequested);
9282
connect(this, &QtTrayMenu::showMenu, this, &QtTrayMenu::onShowMenu);
9383

9484
updateMenu(tray->menu);
9585

96-
trayIcon->setContextMenu(trayTopMenu);
86+
trayIcon->setContextMenu(trayTopMenu.get());
9787
trayIcon->show();
9888

9989
if (notification) {
@@ -147,14 +137,12 @@ void QtTrayMenu::onExitRequested() {
147137
if (trayIcon) {
148138
trayIcon->setContextMenu(nullptr);
149139
}
150-
delete trayTopMenu; // NOSONAR(cpp:S5025): Qt has its own integrated memory management
151-
trayTopMenu = nullptr; // Set to nullptr after deletion
140+
trayTopMenu.reset();
152141
}
153142
// Remove tray icon references;
154143
if (trayIcon) {
155144
trayIcon->hide();
156-
delete trayIcon; // NOSONAR(cpp:S5025): Qt has its own integrated memory management
157-
trayIcon = nullptr; // Set to nullptr after deletion
145+
trayIcon.reset();
158146
}
159147
// Unset tray structure
160148
trayStruct = nullptr;
@@ -167,30 +155,24 @@ void QtTrayMenu::onExitRequested() {
167155

168156
void QtTrayMenu::updateMenu(struct tray_menu *items) {
169157
// Create and setup new tray menu instance
170-
const auto newTrayTopMenu = new QMenu(); // NOSONAR(cpp:S5025): Qt has its own integrated memory management
158+
auto newTrayTopMenu = std::make_unique<QMenu>();
171159
#if defined(_WIN32)
172-
connect(newTrayTopMenu, &QMenu::aboutToShow, this, []() {
160+
connect(newTrayTopMenu.get(), &QMenu::aboutToShow, this, []() {
173161
tray_qt::windows::sync_color_scheme();
174162
});
175163
#endif
176-
trayIcon->setContextMenu(newTrayTopMenu);
164+
trayIcon->setContextMenu(newTrayTopMenu.get());
177165
// Fill new tray menu instance
178-
createMenu(items, newTrayTopMenu);
179-
// Clear old, unused trayTopMenu instance
180-
if (trayTopMenu != nullptr) {
181-
trayTopMenu->clear(); // Remove all actions
182-
delete trayTopMenu; // NOSONAR(cpp:S5025): Qt has its own integrated memory management
183-
}
184-
// Store reference for cleanup
185-
trayTopMenu = newTrayTopMenu;
166+
createMenu(items, newTrayTopMenu.get());
167+
trayTopMenu = std::move(newTrayTopMenu);
186168
}
187169

188170
void QtTrayMenu::createMenu(struct tray_menu *items, QMenu *menu) {
189171
while (items && items->text) {
190172
if (strcmp(items->text, "-") == 0) {
191173
menu->addSeparator();
192174
} else {
193-
auto *action = new QAction(QString::fromUtf8(items->text), menu); // NOSONAR(cpp:S5025): Qt has its own integrated memory management
175+
auto *action = menu->addAction(QString::fromUtf8(items->text));
194176
action->setDisabled(items->disabled == 1);
195177
action->setCheckable(items->checkbox == 1);
196178
action->setChecked(items->checked == 1);
@@ -257,7 +239,7 @@ void QtTrayMenu::onMenuItemTriggered() {
257239
}
258240
}
259241

260-
struct tray_menu *QtTrayMenu::getTrayMenuItem(QAction *action) { // NOSONAR(cpp:S995): Use as defined in function interface
242+
struct tray_menu *QtTrayMenu::getTrayMenuItem(const QAction *action) {
261243
return static_cast<struct tray_menu *>(action->property("tray_menu_item").value<void *>());
262244
}
263245

src/QtTrayMenu.h

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@
55
#ifndef TRAYMENU_H
66
#define TRAYMENU_H
77

8+
// standard includes
9+
#include <array>
10+
#include <memory>
11+
812
// qt includes
913
#include <QMenu>
1014
#include <QObject>
@@ -135,13 +139,17 @@ class QtTrayMenu: public QObject {
135139
void createNotification();
136140
void updateMenu(struct tray_menu *items);
137141
QIcon lookupIcon(QString icon) const;
142+
int defaultArgc = 1;
143+
std::array<char, 12> defaultArgv0 {'T', 'r', 'a', 'y', 'M', 'e', 'n', 'u', 'A', 'p', 'p', '\0'};
144+
std::array<char *, 2> defaultArgv {defaultArgv0.data(), nullptr};
145+
std::unique_ptr<QApplication> ownedApp;
138146
QApplication *app = nullptr;
139-
QSystemTrayIcon *trayIcon = nullptr;
140-
QMenu *trayTopMenu = nullptr;
147+
std::unique_ptr<QSystemTrayIcon> trayIcon;
148+
std::unique_ptr<QMenu> trayTopMenu;
141149
struct tray *trayStruct = nullptr;
142150
bool running = false;
143151
bool blockingEventLoop = false;
144-
struct tray_menu *getTrayMenuItem(QAction *action);
152+
struct tray_menu *getTrayMenuItem(const QAction *action);
145153
mutable std::function<void()> notificationCallback = nullptr;
146154

147155
private slots:

src/tray_qt.cpp

Lines changed: 49 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -19,45 +19,41 @@
1919

2020
namespace tray_qt {
2121
/**
22-
* QtTrayMenu instance
22+
* @brief Process-wide state backing the C tray API.
2323
*/
24-
std::unique_ptr<QtTrayMenu> qt_tray_menu = nullptr; // NOSONAR(cpp:S5421): mutable state, not const
25-
/**
26-
* Logging callback for qt_message_handler
27-
*/
28-
void (*log_callback)(int, const char *) = nullptr; // NOSONAR(cpp:S5421): mutable state, not const
29-
/**
30-
* Explicit Qt application metadata configured through the C API.
31-
*/
32-
bool app_info_configured = false; // NOSONAR(cpp:S5421): mutable state, not const
33-
/**
34-
* Qt application name configured through the C API.
35-
*/
36-
QString app_name; // NOSONAR(cpp:S5421): mutable state, not const
37-
/**
38-
* Qt application display name configured through the C API.
39-
*/
40-
QString app_display_name; // NOSONAR(cpp:S5421): mutable state, not const
24+
struct State {
25+
std::unique_ptr<QtTrayMenu> trayMenu; ///< Active tray menu instance.
26+
void (*logCallback)(int, const char *) = nullptr; ///< Registered C logging callback.
27+
bool appInfoConfigured = false; ///< Whether application metadata was explicitly configured.
28+
QString appName; ///< Configured application name.
29+
QString appDisplayName; ///< Configured application display name.
30+
QString desktopName; ///< Configured desktop file name.
31+
};
32+
4133
/**
42-
* Qt desktop file name configured through the C API.
34+
* @brief Access the process-wide tray API state.
35+
* @return Mutable tray API state.
4336
*/
44-
QString desktop_name; // NOSONAR(cpp:S5421): mutable state, not const
37+
State &state() {
38+
static State instance;
39+
return instance;
40+
}
4541

4642
/**
4743
* @brief Acknowledge/click current notification.
4844
*/
4945
void acknowledge_notification() {
50-
if (qt_tray_menu != nullptr && QtTrayMenu::supportsMessages()) {
51-
qt_tray_menu->clickMessage();
46+
if (state().trayMenu != nullptr && QtTrayMenu::supportsMessages()) {
47+
state().trayMenu->clickMessage();
5248
}
5349
}
5450

5551
/**
5652
* @brief Clear current notification state without invoking callbacks.
5753
*/
5854
void clear_notification() {
59-
if (qt_tray_menu != nullptr) {
60-
qt_tray_menu->clearMessageCallback();
55+
if (state().trayMenu != nullptr) {
56+
state().trayMenu->clearMessageCallback();
6157
}
6258
}
6359

@@ -70,11 +66,11 @@ namespace tray_qt {
7066
clear_notification();
7167
return;
7268
}
73-
if (qt_tray_menu != nullptr && QtTrayMenu::supportsMessages()) {
69+
if (state().trayMenu != nullptr && QtTrayMenu::supportsMessages()) {
7470
if (tray->notification_icon != nullptr) {
75-
qt_tray_menu->showMessage(tray->notification_title, tray->notification_text, tray->notification_icon, tray->notification_cb);
71+
state().trayMenu->showMessage(tray->notification_title, tray->notification_text, tray->notification_icon, tray->notification_cb);
7672
} else {
77-
qt_tray_menu->showMessage(tray->notification_title, tray->notification_text, tray->notification_cb);
73+
state().trayMenu->showMessage(tray->notification_title, tray->notification_text, tray->notification_cb);
7874
}
7975
}
8076
}
@@ -84,14 +80,15 @@ namespace tray_qt {
8480
* @param allow_defaults Whether empty app info values should apply fallback defaults.
8581
*/
8682
void apply_app_info(const bool allow_defaults = true) {
87-
if (!app_info_configured || qt_tray_menu == nullptr) {
83+
const auto &current_state = state();
84+
if (!current_state.appInfoConfigured || current_state.trayMenu == nullptr) {
8885
return;
8986
}
90-
if (!allow_defaults && app_name.isEmpty() && app_display_name.isEmpty()) {
87+
if (!allow_defaults && current_state.appName.isEmpty() && current_state.appDisplayName.isEmpty()) {
9188
return;
9289
}
9390

94-
qt_tray_menu->configureAppMetadata(app_name, app_display_name, desktop_name);
91+
current_state.trayMenu->configureAppMetadata(current_state.appName, current_state.appDisplayName, current_state.desktopName);
9592
}
9693

9794
/**
@@ -114,7 +111,7 @@ namespace tray_qt {
114111
* @param msg The message string.
115112
*/
116113
void qt_message_handler(QtMsgType type, const QMessageLogContext &, const QString &msg) {
117-
if (log_callback == nullptr) {
114+
if (state().logCallback == nullptr) {
118115
return;
119116
}
120117
int level;
@@ -132,29 +129,31 @@ namespace tray_qt {
132129
level = 3;
133130
break;
134131
}
135-
log_callback(level, msg.toUtf8().constData());
132+
state().logCallback(level, msg.toUtf8().constData());
136133
}
137134
} // namespace tray_qt
138135

139136
extern "C" {
140137
void tray_set_app_info(const char *app_name, const char *app_display_name, const char *desktop_name) {
141-
tray_qt::app_info_configured = true;
142-
tray_qt::app_name = app_name != nullptr ? QString::fromUtf8(app_name) : QString();
143-
tray_qt::app_display_name = app_display_name != nullptr ? QString::fromUtf8(app_display_name) : QString();
144-
tray_qt::desktop_name = desktop_name != nullptr ? QString::fromUtf8(desktop_name) : QString();
138+
auto &state = tray_qt::state();
139+
state.appInfoConfigured = true;
140+
state.appName = app_name != nullptr ? QString::fromUtf8(app_name) : QString();
141+
state.appDisplayName = app_display_name != nullptr ? QString::fromUtf8(app_display_name) : QString();
142+
state.desktopName = desktop_name != nullptr ? QString::fromUtf8(desktop_name) : QString();
145143

146144
tray_qt::apply_app_info();
147145
}
148146

149147
int tray_init(struct tray *tray) {
150-
if (tray_qt::qt_tray_menu == nullptr) {
148+
auto &state = tray_qt::state();
149+
if (state.trayMenu == nullptr) {
151150
tray_qt::configure_platform();
152151
// Create a new unique pointer to QtTrayMenu instance
153-
tray_qt::qt_tray_menu = std::make_unique<QtTrayMenu>();
152+
state.trayMenu = std::make_unique<QtTrayMenu>();
154153
tray_qt::apply_app_info(false);
155154
}
156155

157-
if (const auto result = tray_qt::qt_tray_menu->init(tray, false); result < 0) {
156+
if (const auto result = state.trayMenu->init(tray, false); result < 0) {
158157
// Tray init failed. Clean up and return error.
159158
tray_exit();
160159
return result;
@@ -173,18 +172,18 @@ extern "C" {
173172
}
174173

175174
int tray_loop(int blocking) {
176-
if (tray_qt::qt_tray_menu == nullptr) {
175+
if (tray_qt::state().trayMenu == nullptr) {
177176
return -1;
178177
}
179-
return tray_qt::qt_tray_menu->loop(blocking);
178+
return tray_qt::state().trayMenu->loop(blocking);
180179
}
181180

182181
void tray_update(struct tray *tray) { // NOSONAR(cpp:S995): C API requires this exact mutable-pointer signature
183-
if (tray_qt::qt_tray_menu == nullptr) {
182+
if (tray_qt::state().trayMenu == nullptr) {
184183
return;
185184
}
186185

187-
auto *const tray_menu = tray_qt::qt_tray_menu.get();
186+
auto *const tray_menu = tray_qt::state().trayMenu.get();
188187
const auto apply_update = [tray_menu, tray]() {
189188
tray_menu->update(tray, false);
190189
tray_qt::notify(tray);
@@ -200,14 +199,14 @@ extern "C" {
200199
}
201200

202201
void tray_exit(void) {
203-
if (tray_qt::qt_tray_menu == nullptr) {
202+
if (tray_qt::state().trayMenu == nullptr) {
204203
return;
205204
}
206-
tray_qt::qt_tray_menu->exit();
205+
tray_qt::state().trayMenu->exit();
207206
}
208207

209208
void tray_set_log_callback(void (*cb)(int level, const char *msg)) { // NOSONAR(cpp:S5205): C API requires a plain function pointer callback type
210-
tray_qt::log_callback = cb;
209+
tray_qt::state().logCallback = cb;
211210
if (cb != nullptr) {
212211
qInstallMessageHandler(tray_qt::qt_message_handler);
213212
} else {
@@ -216,17 +215,17 @@ extern "C" {
216215
}
217216

218217
void tray_show_menu(void) {
219-
if (tray_qt::qt_tray_menu == nullptr) {
218+
if (tray_qt::state().trayMenu == nullptr) {
220219
return;
221220
}
222-
tray_qt::qt_tray_menu->showMenu();
221+
tray_qt::state().trayMenu->showMenu();
223222
}
224223

225224
void tray_simulate_menu_item_click(int index) {
226-
if (tray_qt::qt_tray_menu == nullptr) {
225+
if (tray_qt::state().trayMenu == nullptr) {
227226
return;
228227
}
229-
tray_qt::qt_tray_menu->clickMenuItem(index);
228+
tray_qt::state().trayMenu->clickMenuItem(index);
230229
}
231230

232231
void tray_simulate_notification_click(void) {

0 commit comments

Comments
 (0)