Hi,
This package has many problems that render it unusable:
- Upstream never supported kernels >= 2.6.24 (2008), there was a "temporary
patch" added in 0.2-3, but it never went upstream. Many following bugs are
actually bugs in this patch or following quick fixes, as this is the only
code path I can test on a today kernel.
- In version 0.2-5 (2020), the change "Fixed buffer size miscalculation bug"
actually broke almost every functions, and nobody seemed to notice it.
Before this change, when reading /sys files, the last \n was removed. After
this change, it is preserved, but then "1" is compared with "1\n" and it
does not match.
- There are memory leaks at many different places, both in original code and
patched version. I started to make a patch about it, I identified more than
20 places in the code where a free or delete_list is missing.
- Buffer size may overflow: terminating null may be added after last byte,
this may never happen because the files in /sys are never big enough.
- Arrays may overflow: array size are checked after the fact.
- API uses global variables, but some API takes a parameter named "globals",
which is not actually global. The "temporary patch" uses this "globals"
parameter to store whether the current kernel is using the old procfs or the
new sysfs interface.
The problem is that some functions do not take this "globals" parameter
while it would be needed to know how to access the kernel information. As a
result:
- The read_acpi_batt function calls read_acpi_battalarm with the wrong value
to read the alarm state (always procfs), so it does not work on kernels
using the sysfs interface.
- The same function calls read_acpi_battstate which tries to work without
this information. It first tries to read the "state file", and if it
fails, it supposes that it should use the sysfs interface.
This happens to "work" because in init_acpi_batt, the "state file" path is
wrong when using the sysfs interface, the path prefix is missing and the
library tries to read /BAT0/present instead of the correct
/sys/class/power_supply/BAT0/present. If the path is fixed, then the
read_acpi_battstate fails, because it tries to use the procfs interface.
I think this problem was introduced in 0.2-4 because the original patch
broke the library interface and a change was done to put the information
in "globals".
- When using the sysfs interface, only the battery status (discharging,
charging, full) is updated by read_acpi_battstate, but not the present rate,
the remaining capacity and the present voltage. As a result, the remaining
percentage is not updated. I think this would be very surprising for a user
to see that the percentage is always the same.
When using the procfs interface, those were updated.
- When using the sysfs interface, there is a FIXME which says: "is rate ==
current here?". If this not the case, then the remaining charge time and
battery time are probably wrong.
- Any error while reading the sysfs files makes the battery unsupported by
libacpi. On my laptop, the "current_now" is not available and return an
error when read, as a result, libacpi would show no battery on my laptop.
- The "temporary patch" does not support thermal zones and fans. Only AC
adapter and batteries.
I discovered those problem while trying to fix the memory leaks in yacpi,
which are actually leaks in libacpi (https://bugs.debian.org/614282).
When trying to fix the problems, I have the impression to uncover new problems
every time. What I think might be done is:
- to drop the procfs support to make the code simpler, 2.6.24 is no longer in
debian since a long time ago,
- to change the API if needed, as yacpi is actually the only program still
using this library.
Upstream author said[1]: "To be honest, I got demotivated following constant
kernel changes so I didn't touch this code in a long time.", so fixing this
package would make Debian the actual upstream.
My intention is not to blame previous authors and contributors, libacpi is
almost 20 years old, open sourcing a code is not a engagement for your whole
life.
Kind regards,
Nicolas.
[1]: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=689657#10