Skip to content

Conversation

@bdebiase
Copy link
Contributor

@bdebiase bdebiase commented Jul 6, 2024

Adds a UPower service to interact with system battery devices. I only implemented properties I thought would be useful, but more can be added later on.

This is my first shot at C++ so any suggestions would be greatly appreciated!

Copy link
Member

@outfoxxed outfoxxed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot!

Looks pretty good overall, just a few UAFs and nitpicks.

@bdebiase bdebiase requested a review from outfoxxed July 8, 2024 16:22
Copy link
Member

@outfoxxed outfoxxed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noticed a few more things while testing.

Here's my tester if you don't have one, but I assume you do

import QtQuick
import QtQuick.Controls
import QtQuick.Layouts
import Quickshell
import Quickshell.Services.UPower

ShellRoot {
	Component {
		id: comp

		Rectangle {
			required property var modelData;

			implicitWidth: layout.implicitWidth + 10
			implicitHeight: layout.implicitHeight + 10
			color: palette.active.base

			ColumnLayout {
				id: layout

				Label { text: `is display device: ${modelData == UPower.displayDevice}` }
				Label { text: `device type: ${UPowerDeviceType.toString(modelData.type)}` }
				Label { text: `is power supply: ${modelData.powerSupply}` }
				Label { text: `energy: ${modelData.energy}` }
				Label { text: `energy capacity: ${modelData.energyCapacity}` }
				Label { text: `change rate: ${modelData.changeRate}` }
				Label { text: `time to empty: ${modelData.timeToEmpty}` }
				Label { text: `time to full: ${modelData.timeToFull}` }
				Label { text: `percentage: ${modelData.percentage}` }
				Label { text: `is present: ${modelData.isPresent}` }
				Label { text: `state: ${UPowerDeviceState.toString(modelData.state)}` }
				Label { text: `health percentage: ${modelData.healthPercentage}` }
				Label { text: `health supported: ${modelData.healthSupported}` }
				Label { text: `icon name: ${modelData.iconName}` }
				Label { text: `is laptop battery: ${modelData.isLaptopBattery}` }
			}
		}
	}

	FloatingWindow {
		color: contentItem.palette.active.window

		ListView {
			anchors.fill: parent
			spacing: 10

			model: UPower.devices
			delegate: comp
		}

		Loader {
			anchors.right: parent.right
			active: UPower.displayDevice != null

			sourceComponent: BoundComponent {
				sourceComponent: comp
				property var modelData: UPower.displayDevice
			}
		}
	}
}

Copy link
Member

@outfoxxed outfoxxed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After this please squash+rebase and it should be good to merge.

@bdebiase bdebiase requested a review from outfoxxed July 10, 2024 18:22
@outfoxxed
Copy link
Member

LGTM, thanks!

@outfoxxed outfoxxed merged commit 24f54f5 into quickshell-mirror:master Jul 10, 2024
@bdebiase bdebiase deleted the service/upower branch July 10, 2024 19:25
@bdebiase
Copy link
Contributor Author

Thanks for everything!

@Actro25 Actro25 mentioned this pull request Sep 18, 2025
da-x added a commit to da-x/quickshell that referenced this pull request Oct 3, 2025
This is because QTextStream is not thread safe, as witnessed by this ASAN catch:

|    #0 0x7ffff78fb648 in realloc.part.0 (/lib64/libasan.so.8+0xfb648) (BuildId: 62576d6cef8744b024a915995656ecd4afeb4351)
|    quickshell-mirror#1 0x7ffff44fa2c7 in QArrayData::reallocateUnaligned(QArrayData*, void*, long long, long long, QArrayData::AllocationOption) (/lib64/libQt6Core.so.6+0x2fa2c7) (BuildId: c21ff2de250ed0aba68ae0250f9a0d1c455f9fd3)
|    quickshell-mirror#2 0x7ffff44d025c in QArrayDataPointer<char16_t>::reallocateAndGrow(QArrayData::GrowthPosition, long long, QArrayDataPointer<char16_t>*) (/lib64/libQt6Core.so.6+0x2d025c) (BuildId: c21ff2de250ed0aba68ae0250f9a0d1c455f9fd3)
|    quickshell-mirror#3 0x7ffff44ca21e in QString::append(QLatin1String) (/lib64/libQt6Core.so.6+0x2ca21e) (BuildId: c21ff2de250ed0aba68ae0250f9a0d1c455f9fd3)
|    quickshell-mirror#4 0x7ffff4484bec in QTextStream::operator<<(QLatin1String) (/lib64/libQt6Core.so.6+0x284bec) (BuildId: c21ff2de250ed0aba68ae0250f9a0d1c455f9fd3)
|    quickshell-mirror#5 0x6dec0e in qs::log::LogMessage::formatMessage(QTextStream&, qs::log::LogMessage const&, bool, bool, QString const&) /home/dan/dev/gh/da-x/quickshell/src/core/logging.cpp:101
|    quickshell-mirror#6 0x6dfaa3 in qs::log::LogManager::messageHandler(QtMsgType, QMessageLogContext const&, QString const&) /home/dan/dev/gh/da-x/quickshell/src/core/logging.cpp:166
|    quickshell-mirror#7 0x7ffff4326319 in qt_message_print(QtMsgType, QMessageLogContext const&, QString const&) (/lib64/libQt6Core.so.6+0x126319) (BuildId: c21ff2de250ed0aba68ae0250f9a0d1c455f9fd3)
|    quickshell-mirror#8 0x7ffff432c2dd in qt_message_output(QtMsgType, QMessageLogContext const&, QString const&) (/lib64/libQt6Core.so.6+0x12c2dd) (BuildId: c21ff2de250ed0aba68ae0250f9a0d1c455f9fd3)
|    quickshell-mirror#9 0x7ffff433e384 in QDebug::~QDebug() (/lib64/libQt6Core.so.6+0x13e384) (BuildId: c21ff2de250ed0aba68ae0250f9a0d1c455f9fd3)
|    quickshell-mirror#10 0x7ffff44dfd55 in QScopedScopeLevelCounter::QScopedScopeLevelCounter(QThreadData*) (/lib64/libQt6Core.so.6+0x2dfd55) (BuildId: c21ff2de250ed0aba68ae0250f9a0d1c455f9fd3)
|    quickshell-mirror#11 0x7ffff43af631 in QCoreApplication::notifyInternal2(QObject*, QEvent*) (/lib64/libQt6Core.so.6+0x1af631) (BuildId: c21ff2de250ed0aba68ae0250f9a0d1c455f9fd3)
|    quickshell-mirror#12 0x7ffff43b344d in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) (/lib64/libQt6Core.so.6+0x1b344d) (BuildId: c21ff2de250ed0aba68ae0250f9a0d1c455f9fd3)
|    quickshell-mirror#13 0x7ffff46e2b86 in postEventSourceDispatch(_GSource*, int (*)(void*), void*) (/lib64/libQt6Core.so.6+0x4e2b86) (BuildId: c21ff2de250ed0aba68ae0250f9a0d1c455f9fd3)
|    quickshell-mirror#14 0x7ffff130cf80 in g_main_context_dispatch_unlocked.lto_priv.0 (/lib64/libglib-2.0.so.0+0x5bf80) (BuildId: 8eb8a9a2c6a8fd0b4f3a00d75e20274c6c52e805)
|    quickshell-mirror#15 0x7ffff136cc67 in g_main_context_iterate_unlocked.isra.0 (/lib64/libglib-2.0.so.0+0xbbc67) (BuildId: 8eb8a9a2c6a8fd0b4f3a00d75e20274c6c52e805)
|    quickshell-mirror#16 0x7ffff130e35f in g_main_context_iteration (/lib64/libglib-2.0.so.0+0x5d35f) (BuildId: 8eb8a9a2c6a8fd0b4f3a00d75e20274c6c52e805)
|    quickshell-mirror#17 0x7ffff46e22f2 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) (/lib64/libQt6Core.so.6+0x4e22f2) (BuildId: c21ff2de250ed0aba68ae0250f9a0d1c455f9fd3)
|    quickshell-mirror#18 0x7ffff43bdcc9 in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) (/lib64/libQt6Core.so.6+0x1bdcc9) (BuildId: c21ff2de250ed0aba68ae0250f9a0d1c455f9fd3)
|    quickshell-mirror#19 0x7ffff44e0c54 in QThread::exec() (/lib64/libQt6Core.so.6+0x2e0c54) (BuildId: c21ff2de250ed0aba68ae0250f9a0d1c455f9fd3)
|    quickshell-mirror#20 0x7ffff458725e in QThreadPrivate::start(void*) (/lib64/libQt6Core.so.6+0x38725e) (BuildId: c21ff2de250ed0aba68ae0250f9a0d1c455f9fd3)
|    quickshell-mirror#21 0x7ffff785e259 in asan_thread_start(void*) (/lib64/libasan.so.8+0x5e259) (BuildId: 62576d6cef8744b024a915995656ecd4afeb4351)
|    quickshell-mirror#22 0x7ffff4cbbb67 in start_thread (/lib64/libc.so.6+0x94b67) (BuildId: 9488b7bc91c299f62d05325020fbee9cfaf229bc)
|    quickshell-mirror#23 0x7ffff4d2c6bb in __clone3 (/lib64/libc.so.6+0x1056bb) (BuildId: 9488b7bc91c299f62d05325020fbee9cfaf229bc)
|
|0x50c00008fe00 is located 0 bytes inside of 128-byte region [0x50c00008fe00,0x50c00008fe80)
|freed by thread T2 (QDBusConnection) here:
|    #0 0x7ffff78fb648 in realloc.part.0 (/lib64/libasan.so.8+0xfb648) (BuildId: 62576d6cef8744b024a915995656ecd4afeb4351)
|    quickshell-mirror#1 0x7ffff44fa2c7 in QArrayData::reallocateUnaligned(QArrayData*, void*, long long, long long, QArrayData::AllocationOption) (/lib64/libQt6Core.so.6+0x2fa2c7) (BuildId: c21ff2de250ed0aba68ae0250f9a0d1c455f9fd3)
|
|previously allocated by thread T1 (QThread) here:
|    #0 0x7ffff78fb648 in realloc.part.0 (/lib64/libasan.so.8+0xfb648) (BuildId: 62576d6cef8744b024a915995656ecd4afeb4351)
|    quickshell-mirror#1 0x7ffff44fa2c7 in QArrayData::reallocateUnaligned(QArrayData*, void*, long long, long long, QArrayData::AllocationOption) (/lib64/libQt6Core.so.6+0x2fa2c7) (BuildId: c21ff2de250ed0aba68ae0250f9a0d1c455f9fd3)
|
|Thread T1 (QThread) created by T0 here:
|    #0 0x7ffff78f40a3 in pthread_create (/lib64/libasan.so.8+0xf40a3) (BuildId: 62576d6cef8744b024a915995656ecd4afeb4351)
|    quickshell-mirror#1 0x7ffff45868e8 in QThread::start(QThread::Priority) (/lib64/libQt6Core.so.6+0x3868e8) (BuildId: c21ff2de250ed0aba68ae0250f9a0d1c455f9fd3)
|    quickshell-mirror#2 0x6e1049 in qs::log::LogManager::init(bool, bool, bool, QtMsgType, QString const&, QString const&) /home/dan/dev/gh/da-x/quickshell/src/core/logging.cpp:261
|    quickshell-mirror#3 0x46a94f in qs::launch::runCommand(int, char**, QCoreApplication*) /home/dan/dev/gh/da-x/quickshell/src/launch/command.cpp:500
|    quickshell-mirror#4 0x445373 in qs::launch::main(int, char**) /home/dan/dev/gh/da-x/quickshell/src/launch/main.cpp:112
|    quickshell-mirror#5 0x4374b5 in main /home/dan/dev/gh/da-x/quickshell/src/main.cpp:3
|    quickshell-mirror#6 0x7ffff4c5130d in __libc_start_call_main (/lib64/libc.so.6+0x2a30d) (BuildId: 9488b7bc91c299f62d05325020fbee9cfaf229bc)
|
|Thread T2 (QDBusConnection) created by T0 here:
|    #0 0x7ffff78f40a3 in pthread_create (/lib64/libasan.so.8+0xf40a3) (BuildId: 62576d6cef8744b024a915995656ecd4afeb4351)
|    quickshell-mirror#1 0x7ffff45868e8 in QThread::start(QThread::Priority) (/lib64/libQt6Core.so.6+0x3868e8) (BuildId: c21ff2de250ed0aba68ae0250f9a0d1c455f9fd3)
|    quickshell-mirror#2 0x7ffff613a166 in QDBusConnectionManager::instance() (/lib64/libQt6DBus.so.6+0x3d166) (BuildId: 35a23e05853374dafc6df118df20e91852bc6344)
|
|SUMMARY: AddressSanitizer: double-free (/lib64/libasan.so.8+0xfb648) (BuildId: 62576d6cef8744b024a915995656ecd4afeb4351) in realloc.part.0
|==3529858==ABORTING
|[Thread 0x7fffd40f56c0 (LWP 3529873) exited]
|[Thread 0x7fffd53ff6c0 (LWP 3529872) exited]
|[Inferior 1 (process 3529858) exited with code 01]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants