Hi,
On 13-07-18 14:36, Andy Shevchenko wrote:
On Thu, 2018-07-12 at 21:20 +0200, Hans de Goede wrote:
Hi,
On 12-07-18 19:23, Andy Shevchenko wrote:
acpi_dev_present() and acpi_dev_get_first_match_name() are missing
put_device() call and thus keeping reference counting unbalanced.
In order to fix the issue introduce a new helper to convert existing
users
one-by-one to a better API.
+bool acpi_dev_present(const char *hid, const char *uid, s64 hrv)
+{
+ struct acpi_device *adev = acpi_dev_get_first_match(hid,
uid, hrv);
+
+ return !!adev;
}
EXPORT_SYMBOL(acpi_dev_present);
Why not just do:
bool acpi_dev_present(const char *hid, const char *uid, s64 hrv)
{
struct acpi_device *adev = acpi_dev_get_first_match(hid, uid,
hrv);
if (!adev)
return false;
put_device(&adev->dev);
return true;
}
And not deprecate this. This fixes the leak while keeping the
API usage simple for users of this API. Having to do a put_device
in all callers seems cumbersome if we can just do it here.
The new API has been dictated by the nature of bus_find_device().
Thus same rules applies.
However, in this very particular case (since we are a) just checking for
presense, and b) don't care if device will gone) it's okay like you
proposed.
So, would you agree on splitting this to several changes for better
granularity, i.e.
- introduce new API
- convert acpi_dev_present() to use it
- fix the issue (squash with previous?)
- ... do similar to acpi_dev_get_first_match_name() ...
?
Yes that is fine with me.
Of course, we can fix acpi_dev_present() right now w/o new API, but it
feels to be half baked without fixing acpi_dev_get_first_match_name().
First fixing acpi_dev_get_first_match_name() is fine with me.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
