On a split gauge/charger the threshold attributes are on the charger, not
the battery, and upstream only looks at the battery — so a settable ceiling
reported as unsupported, and a write went to a path that does not exist.
Same supplier walk charge_type already uses. Also report the live value when
no hwdb CHARGE_LIMIT entry recommends a pair.
Upstream ships no CI in this tree, so the gate is meson's own contract:
configure, build, test, with the optional introspection and idevice halves
switched off so every run proves the same thing.
Two bugs, same shape as charge-type.
The heuristic assumed the percentage is derived from charge_full/energy_full.
When the kernel reports capacity directly - which is where upower actually
reads the percentage from - it is not derived from anything, so the
wrong-units failure the check guards against cannot occur. qcom-battery
reports capacity and no charge_full at all, so it failed a test against a
value it never uses and a good 56% reading was marked untrusted.
And like charge-type, the property was only ever set in UpDeviceSupply, so a
real battery reported the FALSE default regardless.
Short-circuit to trusted when capacity is present and in range; set the
property in UpDeviceSupplyBattery alongside charge-type.
up-enumerator-udev.c tries UP_TYPE_DEVICE_SUPPLY_BATTERY first for every
power_supply device and only falls back to UP_TYPE_DEVICE_SUPPLY if that
fails to initialise, so a system battery is backed by UpDeviceSupplyBattery
and never runs UpDeviceSupply::refresh_device. charge-type was only ever set
there, so it sat at the UNKNOWN enum default - indistinguishable from the
driver not reporting it.
Verified on blueline: with 718e921 installed and the supplier walk correct,
upower still read charge-type: unknown while the charger read Fast.
Note charge_type (active regime) is not charge_types (settable profile list
already handled here for charge thresholds).
On a discrete-PMIC platform the fuel gauge and the charger are separate
devices and only the charger knows the charge regime. On SDM845
qcom-battery is fuel-gauge@4000 and exposes no charge_type at all, while
pmi8998-charger is charger@1000 and does, so reading only the battery's
own attribute returned UNKNOWN on every refresh.
Follow the device links the kernel already publishes rather than guessing
which sibling supply is the charger:
<native>/device/supplier:* -> /sys/devices/virtual/devlink/<s>--<c>
<devlink>/supplier -> the supplying device
<supplier>/power_supply/<name>/charge_type
The battery's own attribute still wins when present, so this is inert on
hardware where the gauge reports charge_type itself.
The kernel prints POWER_SUPPLY_CHARGE_TYPE_NONE as "N/A", not "None",
so up_device_charge_type_from_string() fell through to the warning
path and returned UNKNOWN instead of NONE on every refresh while
discharging.
Stock UPower maps the kernel power_supply 'status' to Device:State but
drops 'charge_type' entirely, so the entire fast/trickle/taper
distinction is invisible over D-Bus. A device reads 'Charging' the
whole constant-voltage tail, which is how a lock surface ends up
showing 'Charging N%' forever on a topped-off pack.
Add ChargeType (mirroring POWER_SUPPLY_CHARGE_TYPE_*) as a separate
uint property, read from sysfs 'charge_type' next to status. Also add
PercentageTrusted: capacity is derived from charge_full/energy_full,
and a driver change reporting wrong units makes a healthy pack read
~1%. Mark it untrusted when full is implausibly small vs design so
consumers can suppress the meaningless number.
Generated skeleton, lib props, Linux backend read, and introspection
XML all wired. Builds clean, self-test passes.
At boot, the Asus battery driver returns -ENODATA [1] to avoid resetting
charge threshold settings. This behavior prevents UPower's udev rules
from detecting the charge threshold feature. This issue can be resolved
by testing for the attribute's existence when matching udev rules, rather
than performing a read operation at boot.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers?id=186bf90Resolves: #347
Signed-off-by: Kate Hsuan <hpa@redhat.com>
Variable 'error' is declared with g_autoptr(GError) but is never used
anywhere in up_polkit_finalize. No function in the body writes to
&error or reads from error.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com
Signed-off-by: Kate Hsuan <hpa@redhat.com>
The return value of g_timeout_add() is discarded instead of being assigned to timer_id.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com
Signed-off-by: Kate Hsuan <hpa@redhat.com>
Variable 'ret_str' is declared with g_autofree but is never assigned a
value or read anywhere in the function up_kbd_backlight_led_event_io.
It remains NULL throughout the function's lifetime.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com
Signed-off-by: Kate Hsuan <hpa@redhat.com>
Variable 'buf_now' is declared and initialized to NULL, freed via
g_free(buf_now), but is never assigned any value or read anywhere in the
function up_kbd_backlight_find. It is completely dead code.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com
Signed-off-by: Kate Hsuan <hpa@redhat.com>
The enum values UP_HISTORY_PROGRESS and UP_HISTORY_LAST_SIGNAL are defined
but never referenced anywhere in the codebase.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com
Signed-off-by: Kate Hsuan <hpa@redhat.com>
Struct members in _UpDeviceSupplyBattery is declared but never referenced
in any function in this file or elsewhere.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com
Signed-off-by: Kate Hsuan <hpa@redhat.com>
g_error() is always fatal (it calls abort() via G_LOG_LEVEL_ERROR), making
'return FALSE' unreachable dead code. The GError allocated by
g_file_set_contents() is also never freed before the process terminates.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com
Signed-off-by: Kate Hsuan <hpa@redhat.com>
The 'history' parameter in up_device_history_filter() is declared but
never referenced.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com
Signed-off-by: Kate Hsuan <hpa@redhat.com>
The 'client' parameter of up_client_get_display_device() is never used in
the function body. Additionally, no g_return_val_if_fail input validation
is performed on the unused parameter, unlike all other public methods in
this file.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com
Signed-off-by: Kate Hsuan <hpa@redhat.com>
g_file_set_contents() writes battery history files using default
permissions (typically 0644 with umask 0022). Since upower runs as a
system daemon, these world-readable files expose per-device
charge/discharge rate and timing patterns to all local users,
which can reveal system usage patterns.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Kate Hsuan <hpa@redhat.com>
g_file_set_contents() creates the state file with permissions
0666 & ~umask. Since UPower runs as a privileged system daemon, the
resulting file permissions depend entirely on the process umask. With a
permissive umask (e.g. 0000), the state file could be world-writable,
allowing unprivileged users to manipulate the charging-threshold-status
file and influence charge threshold behavior on next daemon startup.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Kate Hsuan <hpa@redhat.com>
In up_device_hid_finalize(), the condition if (hid->priv->fd > 0) would
fail to close file descriptor 0, which is a valid fd. The sentinel value is
-1 (set in up_device_hid_init), so the correct guard is >= 0.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Kate Hsuan <hpa@redhat.com>
In up_device_wup_finalize(), the condition if (wup->priv->fd > 0) would
fail to close file descriptor 0, which is a valid fd. The sentinel value is
-1 (set in up_device_wup_init), so the correct guard is >= 0.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Kate Hsuan <hpa@redhat.com>
In up_input_str_to_bitmask, the 'max_size' parameter (sizeof(bitmask) in bytes)
is passed directly as the max_tokens argument to g_strsplit(). On a 64-bit system
with SW_MAX=0x10, the bitmask array has NBITS(SW_MAX)=1 element (8 bytes),
but g_strsplit() is told it can return up to 8 tokens. The loop writes bitmask[j]
where j increments from 0 to g_strv_length(v)-1, so if the input string contains
more than 1 space-separated token, writes to bitmask[1] through bitmask[7]
overflow the array.
1. Extract max_elements variable and add explicit bounds check (j < max_elements)
in the loop condition. While g_strsplit already limits the token count, an explicit
guard prevents out-of-bounds writes if g_strsplit behavior changes or the logic is
refactored, providing defense-in-depth.
2. The variable v is managed by Glib auto clean up.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Kate Hsuan <hpa@redhat.com>
- Move the GMainLoop creation to the code path of the monitor function.
- Quit GMainLoop when receiving a signal.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Security Scanner <scanner@automated>
Free GPtrArray with Glib auto cleanup to prevent memory leak.
Co-authored by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Kate Hsuan <hpa@redhat.com>
Global pointer 'history_dir' is allocated via g_build_filename()
and modified in-place by mkdtemp(), but is never freed with g_free().
After rmdir(history_dir), the allocated string is leaked when the
test returns through g_test_run().
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Kate Hsuan <hpa@redhat.com>
Avoid a potential crash in UPower by returning immediately if the
GetHistory() D-Bus call is invoked with a resolution value of 0.
When the resolution is 0, UPower crashes while calculating the
expected number of history records. Short-circuiting this call
prevents the invalid calculation entirely.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Kate Hsuan <hpa@redhat.com>
g_ptr_array_new() is called without a free function, but items
are added to the list via g_object_ref(). When this array is later freed
with g_ptr_array_unref() in up_history_get_data(), the referenced
GObjects are never unreferenced, leaking memory on every call to
up_history_get_data() with a non-zero timespan.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Kate Hsuan <hpa@redhat.com>
Replace manual g_free() with g_autofree to ensure the memory freed
when the function returns;
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Kate Hsuan <hpa@redhat.com>
In up_device_bluez_coldplug(), when the 'Appearance' property exists but
its value is 0, the GVariant 'v' is non-NULL. The if condition at line 186
evaluates to false (because the value is 0), and execution falls through to
the else-if at line 192, which overwrites 'v' with a new
g_dbus_proxy_get_cached_property(). The original GVariant from the
'Appearance' property is never unreffed.
The `g_autoptr (GVariant)` is used to ensure the allocated GVariant can be
autometically freed when up_device_bluez_coldplug() returns.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Kate Hsuan <hpa@redhat.com>
up_backend_finalize does not free backend->priv->udev_enum. The field is
allocated in up_backend_coldplug via
g_object_new(UP_TYPE_ENUMERATOR_UDEV, ...) in up_backend_coldplug() and
freed in up_backend_unplug(), but the GObject finalize handler omits it.
If the object is destroyed without up_backend_unplug being called first,
udev_enum leaks.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Kate Hsuan <hpa@redhat.com>
The main changes:
- add the meson-build.sh helper script because it produces nicer output
and more consistent behavior across jobs. Use that script together with
MESON_ARGS as variables so the per-job configuration is easier
visible.
- use the FDO_* variables for debian too and rely on ci-templates for
images based on those variables
- always create containers, the ci-templates will skip creating those
anyway if they already exist
- always install the extra deps we need for libgudev, this removes our
exposure to distribution updates changing our test environment
- use a parallel:matrix for the backend jobs
- debian no longer installs the build-dep, let's use explicitly listed
deps only
There is no more real need for scheduled pipelines, so the checks for
those have been removd.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Replace the custom signed-off-by job with one used across multiple fdo
projects. This one also checks a few more things including subject
lengths etc.
We don't want to run it on master so it doesn't fail after merging for
whatever reason (any error should be caught by the MR pipeline anyway).
And we need a higher git depth so ci-fairy can find the merge base.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Pulling in master means our CI may change as ci-templates change (and
exposes us to possible breakages/regressions). Let's pin a sha and only
update it when we actually have to.
And combine the two files from the same include so we don't need to
duplicate the sha.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Add a Signed-off-by check in the pre-commit stage when submitting the
merge request. If the Signed-off-by can't be found in the commit
message, git-signoff.py returns an error and lists all the commits without
the tag.
Co-authored-by: Cursor
Co-authored by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Kate Hsuan <hpa@redhat.com>
When disabling the charging threshold feature upower will change the
charging strategy from "Long Life" to "Fast" if a battery supports
"Long Life", "Standard" and "Fast". This behavior is suboptimal
because "Fast" charging might wear down the battery over time, while
"Standard" is supposed to be used as the safe default.
Fix this by preferring "Standard" over "Fast" charging in such
situations.
Resolves: #344
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
- Feature: Skip the systemd inhibitor when performing CriticalPowerAction
(!309)
- Feature: Introduce "Auto" CriticalPowerAction using systemd-logind
Sleep() (!309)
- Fix: Test CanPowerOff() availability before calling PowerOff() (!311)
- Fix: Add charge limit support for systems providing only
charge_control_end_threshold (!310, #342, #285)
Co-Authored-By: Cursor and Claude-4.6-sonnet
Signed-off-by: Kate Hsuan <hpa@redhat.com>
Call CanPowerOff() to find the availability before calling PowerOff().
The priority of the Sleep() call should be higher than Ignore. That is the
final fallback call when the other critical power actions aren't
available.
Signed-off-by: Kate Hsuan <hpa@redhat.com>
