* [PATCH BlueZ RFC] Refactor uid_state handling
@ 2025-06-20 11:31 Andrew Sayers
2025-06-20 12:54 ` [BlueZ,RFC] " bluez.test.bot
2025-06-20 13:59 ` [PATCH BlueZ RFC] " Luiz Augusto von Dentz
0 siblings, 2 replies; 3+ messages in thread
From: Andrew Sayers @ 2025-06-20 11:31 UTC (permalink / raw)
To: linux-bluetooth; +Cc: luiz.dentz, Andrew Sayers
We talked recently about refactoring logind.{c,h} to fit more neatly
into the program's wider architecture. This patch sketches out a
possible approach. It compiles, but has not been tested beyond that.
If I'm on the right track, I'll come back with a proper solution.
The old API provided callbacks on high-level "init" and "exit" events.
That made sense for the limited case it was used for, but seems
unlikely to scale if this starts picking up more use cases. So the
RFC API just provides a single event that passes a `logind_cb_context`
struct to callbacks, and provides `LOGIND_USER_IS_ACTIVE(ctxt)` to
replicate the old behaviour.
The old API invited individual transports/drivers to use it directly,
which again made sense for version 1, but bypassed `transport.h` and
`driver.h`. The RFC API registers callbacks with `driver.h` and
`transport.h` in a more normal way, and they pass `logind_cb_context`
straight through to the callbacks. That means we won't need to do
much refactoring if the struct changes some day, but also means
individual transports/drivers need to know about `logind.h`.
The alternative would look something like this:
--- obexd/client/driver.h
+++ obexd/client/driver.h
struct obc_driver {
const char *service;
const char *uuid;
void *target;
gsize target_len;
void *(*supported_features) (struct obc_session *session);
int (*probe) (struct obc_session *session);
void (*remove) (struct obc_session *session);
+ int (*uid_state) (const char *state, const int seats);
};
--- obexd/client/driver.c
+++ obexd/client/driver.c
+static void call_cb(gpointer data, gpointer ctxt_)
+{
+ struct obc_driver *driver = (struct obc_driver *)data;
+ struct logind_cb_context *ctxt = (struct logind_cb_context *)ctxt_;
+ if (driver->uid_state) {
+ ctxt->res |= driver->uid_state(ctxt->state, ctxt->seats);
+ }
+}
That would have a better best case (fewer `#include logind.h`s strewn
throughout the code) but a worse worst case (bigger refactoring job if
we need to pass an extra argument some day). I think the RFC patch is
a better balance of risks, but am happy either way.
The RFC patches for `pbap.c` and `bluetooth.c` are designed to show
the difference between the old and new API. A proper patch will
probably get rid of the `_cb()`, `_init()` and `_exit()` functions.
Suggested-by: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Signed-off-by: Andrew Sayers <kernel.org@pileofstuff.org>
---
obexd/client/driver.c | 18 +++++
obexd/client/driver.h | 5 ++
obexd/client/pbap.c | 62 ++++++++++------
obexd/plugins/bluetooth.c | 30 ++++++--
obexd/src/logind.c | 152 ++++++++++++++++++--------------------
obexd/src/logind.h | 59 ++++++++++++---
obexd/src/main.c | 12 +++
obexd/src/transport.c | 17 +++++
obexd/src/transport.h | 5 ++
9 files changed, 237 insertions(+), 123 deletions(-)
diff --git a/obexd/client/driver.c b/obexd/client/driver.c
index 195cdd2f1..d5d0fe2ab 100644
--- a/obexd/client/driver.c
+++ b/obexd/client/driver.c
@@ -74,3 +74,21 @@ void obc_driver_unregister(struct obc_driver *driver)
drivers = g_slist_remove(drivers, driver);
}
+
+static void call_cb(gpointer data, gpointer ctxt)
+{
+ struct obc_driver *driver = (struct obc_driver *)data;
+
+ if (driver->uid_state)
+ driver->uid_state(((struct logind_cb_context *)ctxt));
+}
+
+static void call_uid_state_cb(gpointer ctxt)
+{
+ g_slist_foreach(drivers, call_cb, ctxt);
+}
+
+gboolean obc_driver_init(void)
+{
+ return logind_register(call_uid_state_cb) >= 0;
+}
diff --git a/obexd/client/driver.h b/obexd/client/driver.h
index cc4cace7b..928c0c558 100644
--- a/obexd/client/driver.h
+++ b/obexd/client/driver.h
@@ -8,6 +8,9 @@
*
*/
+#include "obexd/src/logind.h"
+struct obc_session;
+
struct obc_driver {
const char *service;
const char *uuid;
@@ -16,8 +19,10 @@ struct obc_driver {
void *(*supported_features) (struct obc_session *session);
int (*probe) (struct obc_session *session);
void (*remove) (struct obc_session *session);
+ void (*uid_state) (struct logind_cb_context *ctxt);
};
int obc_driver_register(struct obc_driver *driver);
void obc_driver_unregister(struct obc_driver *driver);
struct obc_driver *obc_driver_find(const char *pattern);
+gboolean obc_driver_init(void);
diff --git a/obexd/client/pbap.c b/obexd/client/pbap.c
index 78c46bf86..dc2a519e5 100644
--- a/obexd/client/pbap.c
+++ b/obexd/client/pbap.c
@@ -146,6 +146,9 @@ static DBusConnection *system_conn;
static unsigned int listener_id;
static char *client_path;
+static int pbap_init_cb(void);
+static void pbap_exit_cb(void);
+
static struct pending_request *pending_request_new(struct pbap_data *pbap,
DBusMessage *message)
{
@@ -1300,6 +1303,20 @@ static void pbap_remove(struct obc_session *session)
g_dbus_unregister_interface(conn, path, PBAP_INTERFACE);
}
+static gboolean user_was_active = FALSE;
+static void pbap_uid_state(struct logind_cb_context *ctxt)
+{
+ gboolean user_is_active = LOGIND_USER_IS_ACTIVE(ctxt);
+
+ if (user_was_active == user_is_active)
+ return;
+ user_was_active = user_is_active;
+ if (user_is_active)
+ pbap_init_cb();
+ else
+ pbap_exit_cb();
+}
+
static DBusMessage *pbap_release(DBusConnection *conn,
DBusMessage *msg, void *data)
{
@@ -1452,13 +1469,13 @@ static struct obc_driver pbap = {
.target_len = OBEX_PBAP_UUID_LEN,
.supported_features = pbap_supported_features,
.probe = pbap_probe,
- .remove = pbap_remove
+ .remove = pbap_remove,
+ .uid_state = pbap_uid_state
};
-static int pbap_init_cb(gboolean at_register)
+static int pbap_init_cb(void)
{
int err;
- (void)at_register;
DBG("");
@@ -1483,7 +1500,7 @@ static int pbap_init_cb(gboolean at_register)
return 0;
}
-static void pbap_exit_cb(gboolean at_unregister)
+static void pbap_exit_cb(void)
{
DBusMessage *msg;
DBusMessageIter iter;
@@ -1491,22 +1508,28 @@ static void pbap_exit_cb(gboolean at_unregister)
DBG("");
- if (!at_unregister) {
- client_path = g_strconcat("/org/bluez/obex/", uuid, NULL);
- g_strdelimit(client_path, "-", '_');
+ client_path = g_strconcat("/org/bluez/obex/", uuid, NULL);
+ g_strdelimit(client_path, "-", '_');
- msg = dbus_message_new_method_call("org.bluez", "/org/bluez",
- "org.bluez.ProfileManager1",
- "UnregisterProfile");
+ msg = dbus_message_new_method_call("org.bluez", "/org/bluez",
+ "org.bluez.ProfileManager1",
+ "UnregisterProfile");
- dbus_message_iter_init_append(msg, &iter);
+ dbus_message_iter_init_append(msg, &iter);
- dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH,
- &client_path);
+ dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH,
+ &client_path);
- g_dbus_send_message(system_conn, msg);
- }
+ g_dbus_send_message(system_conn, msg);
+ pbap_exit();
+}
+
+int pbap_init(void)
+{
+}
+void pbap_exit(void)
+{
g_dbus_remove_watch(system_conn, listener_id);
unregister_profile();
@@ -1524,12 +1547,3 @@ static void pbap_exit_cb(gboolean at_unregister)
obc_driver_unregister(&pbap);
}
-
-int pbap_init(void)
-{
- return logind_register(pbap_init_cb, pbap_exit_cb);
-}
-void pbap_exit(void)
-{
- return logind_unregister(pbap_init_cb, pbap_exit_cb);
-}
diff --git a/obexd/plugins/bluetooth.c b/obexd/plugins/bluetooth.c
index 355921479..ebff88db3 100644
--- a/obexd/plugins/bluetooth.c
+++ b/obexd/plugins/bluetooth.c
@@ -57,6 +57,9 @@ static DBusMessage *profile_release(DBusConnection *conn, DBusMessage *msg,
return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
}
+static int bluetooth_init_cb(void);
+static void bluetooth_exit_cb(void);
+
static void connect_event(GIOChannel *io, GError *err, void *user_data)
{
int sk = g_io_channel_unix_get_fd(io);
@@ -381,6 +384,20 @@ static void bluetooth_stop(void *data)
profiles = NULL;
}
+static gboolean user_was_active = FALSE;
+static void bluetooth_uid_state(struct logind_cb_context *ctxt)
+{
+ gboolean user_is_active = LOGIND_USER_IS_ACTIVE(ctxt);
+
+ if (user_was_active == user_is_active)
+ return;
+ user_was_active = user_is_active;
+ if (user_is_active)
+ bluetooth_init_cb();
+ else
+ bluetooth_exit_cb();
+}
+
static int bluetooth_getpeername(GIOChannel *io, char **name)
{
GError *gerr = NULL;
@@ -422,14 +439,14 @@ static const struct obex_transport_driver driver = {
.start = bluetooth_start,
.getpeername = bluetooth_getpeername,
.getsockname = bluetooth_getsockname,
- .stop = bluetooth_stop
+ .stop = bluetooth_stop,
+ .uid_state = bluetooth_uid_state
};
static unsigned int listener_id = 0;
-static int bluetooth_init_cb(gboolean at_register)
+static int bluetooth_init_cb(void)
{
- (void)at_register;
connection = g_dbus_setup_private(DBUS_BUS_SYSTEM, NULL, NULL);
if (connection == NULL)
return -EPERM;
@@ -440,10 +457,9 @@ static int bluetooth_init_cb(gboolean at_register)
return obex_transport_driver_register(&driver);
}
-static void bluetooth_exit_cb(gboolean at_unregister)
+static void bluetooth_exit_cb(void)
{
GSList *l;
- (void)at_unregister;
g_dbus_remove_watch(connection, listener_id);
@@ -467,11 +483,11 @@ static void bluetooth_exit_cb(gboolean at_unregister)
static int bluetooth_init(void)
{
- return logind_register(bluetooth_init_cb, bluetooth_exit_cb);
}
static void bluetooth_exit(void)
{
- return logind_unregister(bluetooth_init_cb, bluetooth_exit_cb);
+ if (user_was_active)
+ bluetooth_exit_cb();
}
OBEX_PLUGIN_DEFINE(bluetooth, bluetooth_init, bluetooth_exit)
diff --git a/obexd/src/logind.c b/obexd/src/logind.c
index b4279b209..a8ea25543 100644
--- a/obexd/src/logind.c
+++ b/obexd/src/logind.c
@@ -27,66 +27,48 @@
static sd_login_monitor * monitor;
static int uid;
-static gboolean active = FALSE;
static gboolean monitoring_enabled = TRUE;
static guint event_source;
static guint timeout_source;
-struct callback_pair {
- logind_init_cb init_cb;
- logind_exit_cb exit_cb;
-};
-
GSList *callbacks;
-static void call_init_cb(gpointer data, gpointer user_data)
-{
- int res;
-
- res = ((struct callback_pair *)data)->init_cb(FALSE);
- if (res)
- *(int *)user_data = res;
-}
-static void call_exit_cb(gpointer data, gpointer user_data)
+static void call_cb(gpointer data, gpointer user_data)
{
- ((struct callback_pair *)data)->exit_cb(FALSE);
+ (*((logind_cb *)data))(user_data);
}
-static int update(void)
+static void logind_cb_context_init(struct logind_cb_context *ctxt)
{
- char *state = NULL;
- gboolean state_is_active;
- int res;
-
- res = sd_login_monitor_flush(monitor);
- if (res < 0)
- return res;
- res = sd_uid_get_state(uid, &state);
- state_is_active = g_strcmp0(state, "active");
- free(state);
- if (res < 0)
- return res;
-
- if (state_is_active) {
- if (!active)
- return 0;
- } else {
- res = sd_uid_get_seats(uid, 1, NULL);
- if (res < 0)
- return res;
- if (active == !!res)
- return 0;
+ ctxt->res = sd_login_monitor_flush(monitor);
+ if (ctxt->res < 0)
+ return;
+
+ ctxt->res = ctxt->seats = sd_uid_get_seats(uid, 1, NULL);
+ if (ctxt->res < 0)
+ return;
+
+ /*
+ * the documentation for sd_uid_get_state() isn't clear about
+ * what to do with the state on error. The following should
+ * be safe even if the behaviour changes in future
+ */
+ ctxt->state = 0;
+ ctxt->res = sd_uid_get_state(uid, (char **)&ctxt->state);
+ if (ctxt->res <= 0) {
+ free((char *)ctxt->state);
+ return;
}
- active ^= TRUE;
- res = 0;
- g_slist_foreach(callbacks, active ? call_init_cb : call_exit_cb, &res);
- return res;
+
+ ctxt->res = 0;
+ return;
}
static gboolean timeout_handler(gpointer user_data);
static int check_event(void)
{
+ struct logind_cb_context ctxt;
uint64_t timeout_usec;
int res;
@@ -95,9 +77,14 @@ static int check_event(void)
return res;
if (!monitoring_enabled)
return 0;
- res = update();
- if (res < 0)
- return res;
+
+ logind_cb_context_init(&ctxt);
+ if (ctxt.res)
+ return ctxt.res;
+ g_slist_foreach(callbacks, call_cb, &ctxt);
+ free((char *)ctxt.state);
+ if (ctxt.res)
+ return ctxt.res;
res = sd_login_monitor_get_timeout(monitor, &timeout_usec);
if (res < 0)
@@ -154,6 +141,7 @@ static gboolean timeout_handler(gpointer user_data)
static int logind_init(void)
{
+ struct logind_cb_context ctxt;
GIOChannel *channel;
int events;
int fd;
@@ -174,11 +162,6 @@ static int logind_init(void)
goto FAIL;
}
- // Check this after creating the monitor, in case of race conditions:
- res = update();
- if (res < 0)
- goto FAIL;
-
events = res = sd_login_monitor_get_events(monitor);
if (res < 0)
goto FAIL;
@@ -202,7 +185,10 @@ static int logind_init(void)
FAIL:
sd_login_monitor_unref(monitor);
monitoring_enabled = FALSE;
- active = TRUE;
+ ctxt.state = "active";
+ ctxt.seats = 1;
+ ctxt.res = 0;
+ g_slist_foreach(callbacks, call_cb, &ctxt);
return res;
}
@@ -219,17 +205,18 @@ static void logind_exit(void)
sd_login_monitor_unref(monitor);
}
-static gint find_cb(gconstpointer a, gconstpointer b)
+int logind_register(logind_cb cb)
{
- return ((struct callback_pair *)a)->init_cb - (logind_init_cb)b;
-}
+ struct logind_cb_context ctxt;
-int logind_register(logind_init_cb init_cb, logind_exit_cb exit_cb)
-{
- struct callback_pair *cbs;
+ logind_cb_context_init(&ctxt);
+ if (ctxt.res) {
+ free((char *)ctxt.state);
+ return ctxt.res;
+ }
if (!monitoring_enabled)
- return init_cb(TRUE);
+ goto CALL_CB;
if (callbacks == NULL) {
int res;
@@ -237,24 +224,23 @@ int logind_register(logind_init_cb init_cb, logind_exit_cb exit_cb)
if (res) {
error("logind_init(): %s - login detection disabled",
strerror(-res));
- return init_cb(TRUE);
+ goto CALL_CB;
}
}
- cbs = g_new(struct callback_pair, 1);
- cbs->init_cb = init_cb;
- cbs->exit_cb = exit_cb;
- callbacks = g_slist_prepend(callbacks, cbs);
- return active ? init_cb(TRUE) : 0;
+ callbacks = g_slist_prepend(callbacks, cb);
+
+CALL_CB:
+ cb(&ctxt);
+ free((char *)ctxt.state);
+ return ctxt.res;
}
-void logind_unregister(logind_init_cb init_cb, logind_exit_cb exit_cb)
+void logind_unregister(logind_cb cb)
{
GSList *cb_node;
if (!monitoring_enabled)
- return exit_cb(TRUE);
- if (active)
- exit_cb(TRUE);
- cb_node = g_slist_find_custom(callbacks, init_cb, find_cb);
+ return;
+ cb_node = g_slist_find(callbacks, cb);
if (cb_node != NULL)
callbacks = g_slist_delete_link(callbacks, cb_node);
if (callbacks == NULL)
@@ -263,20 +249,26 @@ void logind_unregister(logind_init_cb init_cb, logind_exit_cb exit_cb)
int logind_set(gboolean enabled)
{
- int res = 0;
-
- if (monitoring_enabled == enabled)
- return 0;
-
monitoring_enabled = enabled;
if (enabled) {
- active = FALSE;
- return update();
+ struct logind_cb_context ctxt;
+
+ logind_cb_context_init(&ctxt);
+ if (ctxt.res)
+ return ctxt.res;
+ g_slist_foreach(callbacks, call_cb, &ctxt);
+ free((char *)ctxt.state);
+ return ctxt.res;
}
- active = TRUE;
- g_slist_foreach(callbacks, call_exit_cb, &res);
- return res;
+ struct logind_cb_context ctxt = {
+ .state = "active",
+ .seats = 1,
+ .res = 0
+ };
+
+ g_slist_foreach(callbacks, call_cb, &ctxt);
+ return ctxt.res;
}
#endif
diff --git a/obexd/src/logind.h b/obexd/src/logind.h
index 3cdb03338..243aa17bd 100644
--- a/obexd/src/logind.h
+++ b/obexd/src/logind.h
@@ -8,30 +8,65 @@
*
*/
-typedef int (*logind_init_cb)(gboolean at_register);
-typedef void (*logind_exit_cb)(gboolean at_unregister);
+#ifndef OBEXD_SRC_LOGIND_H
+#define OBEXD_SRC_LOGIND_H
+
+struct logind_cb_context {
+ const char *state;
+ int seats;
+ int res;
+};
+
+typedef void (*logind_cb)(gpointer ctxt);
#ifdef SYSTEMD
-int logind_register(logind_init_cb init_cb, logind_exit_cb exit_cb);
-void logind_unregister(logind_init_cb init_cb, logind_exit_cb exit_cb);
+/*
+ * Register callback and call it with the current state
+ */
+int logind_register(logind_cb init_cb);
+/*
+ * Unregister callback but DO NOT call it -
+ * unregistration usually happens when the user is logging out,
+ * and other programs are going away.
+ *
+ * If possible, close resources at exit instead of at unregister.
+ * Otherwise, you will need to explicitly call your callback.
+ */
+void logind_unregister(logind_cb cb);
+/*
+ * Override the detected login state
+ */
int logind_set(gboolean enabled);
-#else
+/* Recommended way to detect (in)activity */
+#define LOGIND_USER_IS_ACTIVE(ctxt) \
+ (!g_strcmp0(ctxt->state, "active") && !!(ctxt->seats))
-static inline int logind_register(logind_init_cb init_cb,
- logind_exit_cb exit_cb)
+#else /* SYSTEMD */
+
+static inline int logind_register(logind_cb cb)
{
- return init_cb(TRUE);
+ (void)cb;
+ struct logind_cb_context ctxt = {
+ .state = "active",
+ .seats = 1,
+ .res = 0
+ };
+ cb(&ctxt);
+ return ctxt.res;
}
-static inline void logind_unregister(logind_init_cb init_cb,
- logind_exit_cb exit_cb)
+static inline void logind_unregister(logind_cb cb)
{
- return exit_cb(TRUE);
+ (void)cb;
}
static inline int logind_set(gboolean enabled)
{
return 0;
}
-#endif
+#define LOGIND_USER_IS_ACTIVE(...) 1
+
+#endif /* SYSTEMD */
+
+#endif /* OBEXD_SRC_LOGIND_H */
diff --git a/obexd/src/main.c b/obexd/src/main.c
index 6837f0d73..116888a78 100644
--- a/obexd/src/main.c
+++ b/obexd/src/main.c
@@ -34,6 +34,9 @@
#include "../client/manager.h"
+#include "../client/driver.h"
+#include "transport.h"
+
#include "log.h"
#include "logind.h"
#include "obexd.h"
@@ -279,6 +282,15 @@ int main(int argc, char *argv[])
if (option_system_bus)
logind_set(FALSE);
+ if (obc_driver_init() == FALSE) {
+ error("manager_init failed");
+ exit(EXIT_FAILURE);
+ }
+ if (obex_transport_driver_init() == FALSE) {
+ error("manager_init failed");
+ exit(EXIT_FAILURE);
+ }
+
DBG("Entering main loop");
main_loop = g_main_loop_new(NULL, FALSE);
diff --git a/obexd/src/transport.c b/obexd/src/transport.c
index 527d9ffce..fdd49b7a2 100644
--- a/obexd/src/transport.c
+++ b/obexd/src/transport.c
@@ -79,3 +79,20 @@ obex_transport_driver_unregister(const struct obex_transport_driver *driver)
drivers = g_slist_remove(drivers, driver);
}
+
+static void call_cb(gpointer data, gpointer ctxt)
+{
+ struct obex_transport_driver *driver =
+ (struct obex_transport_driver *)data;
+ if (driver->uid_state)
+ driver->uid_state(((struct logind_cb_context *)ctxt));
+}
+
+static int call_uid_state_cb(gpointer ctxt)
+{
+ g_slist_foreach(drivers, call_cb, ctxt);
+}
+
+gboolean obex_transport_driver_init(void)
+{
+}
diff --git a/obexd/src/transport.h b/obexd/src/transport.h
index 322d8f526..087d8c211 100644
--- a/obexd/src/transport.h
+++ b/obexd/src/transport.h
@@ -8,6 +8,9 @@
*
*/
+#include "obexd/src/logind.h"
+struct obex_server;
+
struct obex_transport_driver {
const char *name;
uint16_t service;
@@ -15,9 +18,11 @@ struct obex_transport_driver {
int (*getpeername) (GIOChannel *io, char **name);
int (*getsockname) (GIOChannel *io, char **name);
void (*stop) (void *data);
+ void (*uid_state) (struct logind_cb_context *ctxt);
};
int obex_transport_driver_register(const struct obex_transport_driver *driver);
void
obex_transport_driver_unregister(const struct obex_transport_driver *driver);
const GSList *obex_transport_driver_list(void);
+gboolean obex_transport_driver_init(void);
--
2.49.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* RE: [BlueZ,RFC] Refactor uid_state handling
2025-06-20 11:31 [PATCH BlueZ RFC] Refactor uid_state handling Andrew Sayers
@ 2025-06-20 12:54 ` bluez.test.bot
2025-06-20 13:59 ` [PATCH BlueZ RFC] " Luiz Augusto von Dentz
1 sibling, 0 replies; 3+ messages in thread
From: bluez.test.bot @ 2025-06-20 12:54 UTC (permalink / raw)
To: linux-bluetooth, kernel.org
[-- Attachment #1: Type: text/plain, Size: 34762 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=974231
---Test result---
Test Summary:
CheckPatch PENDING 0.23 seconds
GitLint PENDING 0.22 seconds
BuildEll PASS 20.01 seconds
BluezMake FAIL 87.53 seconds
MakeCheck FAIL 3112.67 seconds
MakeDistcheck PASS 200.47 seconds
CheckValgrind FAIL 64.75 seconds
CheckSmatch FAIL 195.32 seconds
bluezmakeextell FAIL 103.05 seconds
IncrementalBuild PENDING 0.23 seconds
ScanBuild FAIL 238.27 seconds
Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:
##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:
##############################
Test: BluezMake - FAIL
Desc: Build BlueZ
Output:
tools/mgmt-tester.c: In function ‘main’:
tools/mgmt-tester.c:12907:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
12907 | int main(int argc, char *argv[])
| ^~~~
obexd/client/pbap.c: In function ‘pbap_init’:
obexd/client/pbap.c:1530:1: error: control reaches end of non-void function [-Werror=return-type]
1530 | }
| ^
cc1: all warnings being treated as errors
make[1]: *** [Makefile:9930: obexd/client/obexd-pbap.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:4690: all] Error 2
##############################
Test: MakeCheck - FAIL
Desc: Run Bluez Make Check
Output:
unit/test-avdtp.c: In function ‘main’:
unit/test-avdtp.c:766:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
766 | int main(int argc, char *argv[])
| ^~~~
unit/test-avrcp.c: In function ‘main’:
unit/test-avrcp.c:989:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
989 | int main(int argc, char *argv[])
| ^~~~
obexd/plugins/bluetooth.c: In function ‘bluetooth_init’:
obexd/plugins/bluetooth.c:486:1: error: no return statement in function returning non-void [-Werror=return-type]
486 | }
| ^
cc1: all warnings being treated as errors
make[1]: *** [Makefile:9580: obexd/plugins/obexd-bluetooth.o] Error 1
make: *** [Makefile:12345: check] Error 2
##############################
Test: CheckValgrind - FAIL
Desc: Run Bluez Make Check with Valgrind
Output:
tools/mgmt-tester.c: In function ‘main’:
tools/mgmt-tester.c:12907:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
12907 | int main(int argc, char *argv[])
| ^~~~
obexd/client/pbap.c: In function ‘pbap_init’:
obexd/client/pbap.c:1530:1: error: control reaches end of non-void function [-Werror=return-type]
1530 | }
| ^
cc1: all warnings being treated as errors
make[1]: *** [Makefile:9930: obexd/client/obexd-pbap.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:12345: check] Error 2
##############################
Test: CheckSmatch - FAIL
Desc: Run smatch tool with source
Output:
src/shared/crypto.c:271:21: warning: Variable length array is used.
src/shared/crypto.c:272:23: warning: Variable length array is used.
src/shared/gatt-helpers.c:768:31: warning: Variable length array is used.
src/shared/gatt-helpers.c:830:31: warning: Variable length array is used.
src/shared/gatt-helpers.c:1323:31: warning: Variable length array is used.
src/shared/gatt-helpers.c:1354:23: warning: Variable length array is used.
src/shared/gatt-server.c:278:25: warning: Variable length array is used.
src/shared/gatt-server.c:618:25: warning: Variable length array is used.
src/shared/gatt-server.c:716:25: warning: Variable length array is used.
src/shared/bap.c:317:25: warning: array of flexible structures
src/shared/bap.c: note: in included file:
./src/shared/ascs.h:88:25: warning: array of flexible structures
src/shared/shell.c: note: in included file (through /usr/include/readline/readline.h):
/usr/include/readline/rltypedefs.h:35:23: warning: non-ANSI function declaration of function 'Function'
/usr/include/readline/rltypedefs.h:36:25: warning: non-ANSI function declaration of function 'VFunction'
/usr/include/readline/rltypedefs.h:37:27: warning: non-ANSI function declaration of function 'CPFunction'
/usr/include/readline/rltypedefs.h:38:29: warning: non-ANSI function declaration of function 'CPPFunction'
src/shared/crypto.c:271:21: warning: Variable length array is used.
src/shared/crypto.c:272:23: warning: Variable length array is used.
src/shared/gatt-helpers.c:768:31: warning: Variable length array is used.
src/shared/gatt-helpers.c:830:31: warning: Variable length array is used.
src/shared/gatt-helpers.c:1323:31: warning: Variable length array is used.
src/shared/gatt-helpers.c:1354:23: warning: Variable length array is used.
src/shared/gatt-server.c:278:25: warning: Variable length array is used.
src/shared/gatt-server.c:618:25: warning: Variable length array is used.
src/shared/gatt-server.c:716:25: warning: Variable length array is used.
src/shared/bap.c:317:25: warning: array of flexible structures
src/shared/bap.c: note: in included file:
./src/shared/ascs.h:88:25: warning: array of flexible structures
src/shared/shell.c: note: in included file (through /usr/include/readline/readline.h):
/usr/include/readline/rltypedefs.h:35:23: warning: non-ANSI function declaration of function 'Function'
/usr/include/readline/rltypedefs.h:36:25: warning: non-ANSI function declaration of function 'VFunction'
/usr/include/readline/rltypedefs.h:37:27: warning: non-ANSI function declaration of function 'CPFunction'
/usr/include/readline/rltypedefs.h:38:29: warning: non-ANSI function declaration of function 'CPPFunction'
tools/mesh-cfgtest.c:1453:17: warning: unknown escape sequence: '\%'
tools/sco-tester.c: note: in included file:
./lib/bluetooth.h:232:15: warning: array of flexible structures
./lib/bluetooth.h:237:31: warning: array of flexible structures
tools/bneptest.c:634:39: warning: unknown escape sequence: '\%'
tools/seq2bseq.c:57:26: warning: Variable length array is used.
tools/obex-client-tool.c: note: in included file (through /usr/include/readline/readline.h):
/usr/include/readline/rltypedefs.h:35:23: warning: non-ANSI function declaration of function 'Function'
/usr/include/readline/rltypedefs.h:36:25: warning: non-ANSI function declaration of function 'VFunction'
/usr/include/readline/rltypedefs.h:37:27: warning: non-ANSI function declaration of function 'CPFunction'
/usr/include/readline/rltypedefs.h:38:29: warning: non-ANSI function declaration of function 'CPPFunction'
android/avctp.c:505:34: warning: Variable length array is used.
android/avctp.c:556:34: warning: Variable length array is used.
unit/test-avrcp.c:373:26: warning: Variable length array is used.
unit/test-avrcp.c:398:26: warning: Variable length array is used.
unit/test-avrcp.c:414:24: warning: Variable length array is used.
android/avrcp-lib.c:1085:34: warning: Variable length array is used.
android/avrcp-lib.c:1583:34: warning: Variable length array is used.
android/avrcp-lib.c:1612:34: warning: Variable length array is used.
android/avrcp-lib.c:1638:34: warning: Variable length array is used.
obexd/client/pbap.c: In function ‘pbap_init’:
obexd/client/pbap.c:1530:1: error: control reaches end of non-void function [-Werror=return-type]
1530 | }
| ^
cc1: all warnings being treated as errors
make[1]: *** [Makefile:9930: obexd/client/obexd-pbap.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:4690: all] Error 2
##############################
Test: bluezmakeextell - FAIL
Desc: Build Bluez with External ELL
Output:
obexd/client/pbap.c: In function ‘pbap_init’:
obexd/client/pbap.c:1530:1: error: control reaches end of non-void function [-Werror=return-type]
1530 | }
| ^
cc1: all warnings being treated as errors
make[1]: *** [Makefile:9930: obexd/client/obexd-pbap.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:4690: all] Error 2
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:
##############################
Test: ScanBuild - FAIL
Desc: Run Scan Build
Output:
src/shared/gatt-client.c:451:21: warning: Use of memory after it is freed
gatt_db_unregister(op->client->db, op->db_id);
^~~~~~~~~~
src/shared/gatt-client.c:696:2: warning: Use of memory after it is freed
discovery_op_complete(op, false, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:996:2: warning: Use of memory after it is freed
discovery_op_complete(op, success, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1102:2: warning: Use of memory after it is freed
discovery_op_complete(op, success, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1296:2: warning: Use of memory after it is freed
discovery_op_complete(op, success, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1361:2: warning: Use of memory after it is freed
discovery_op_complete(op, success, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1636:6: warning: Use of memory after it is freed
if (read_db_hash(op)) {
^~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1641:2: warning: Use of memory after it is freed
discover_all(op);
^~~~~~~~~~~~~~~~
src/shared/gatt-client.c:2147:6: warning: Use of memory after it is freed
if (read_db_hash(op)) {
^~~~~~~~~~~~~~~~
src/shared/gatt-client.c:2155:8: warning: Use of memory after it is freed
discovery_op_ref(op),
^~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:3180:2: warning: Use of memory after it is freed
complete_write_long_op(req, success, 0, false);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:3202:2: warning: Use of memory after it is freed
request_unref(req);
^~~~~~~~~~~~~~~~~~
12 warnings generated.
src/shared/bap.c:1531:8: warning: Use of memory after it is freed
bap = bt_bap_ref_safe(bap);
^~~~~~~~~~~~~~~~~~~~
src/shared/bap.c:2312:20: warning: Use of memory after it is freed
return queue_find(stream->bap->streams, NULL, stream);
^~~~~~~~~~~~~~~~~~~~
2 warnings generated.
src/shared/gatt-client.c:451:21: warning: Use of memory after it is freed
gatt_db_unregister(op->client->db, op->db_id);
^~~~~~~~~~
src/shared/gatt-client.c:696:2: warning: Use of memory after it is freed
discovery_op_complete(op, false, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:996:2: warning: Use of memory after it is freed
discovery_op_complete(op, success, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1102:2: warning: Use of memory after it is freed
discovery_op_complete(op, success, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1296:2: warning: Use of memory after it is freed
discovery_op_complete(op, success, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1361:2: warning: Use of memory after it is freed
discovery_op_complete(op, success, att_ecode);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1636:6: warning: Use of memory after it is freed
if (read_db_hash(op)) {
^~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1641:2: warning: Use of memory after it is freed
discover_all(op);
^~~~~~~~~~~~~~~~
src/shared/gatt-client.c:2147:6: warning: Use of memory after it is freed
if (read_db_hash(op)) {
^~~~~~~~~~~~~~~~
src/shared/gatt-client.c:2155:8: warning: Use of memory after it is freed
discovery_op_ref(op),
^~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:3180:2: warning: Use of memory after it is freed
complete_write_long_op(req, success, 0, false);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:3202:2: warning: Use of memory after it is freed
request_unref(req);
^~~~~~~~~~~~~~~~~~
12 warnings generated.
tools/hciattach.c:817:7: warning: Although the value stored to 'n' is used in the enclosing expression, the value is never actually read from 'n'
if ((n = read_hci_event(fd, resp, 10)) < 0) {
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/hciattach.c:865:7: warning: Although the value stored to 'n' is used in the enclosing expression, the value is never actually read from 'n'
if ((n = read_hci_event(fd, resp, 4)) < 0) {
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/hciattach.c:887:8: warning: Although the value stored to 'n' is used in the enclosing expression, the value is never actually read from 'n'
if ((n = read_hci_event(fd, resp, 10)) < 0) {
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/hciattach.c:909:7: warning: Although the value stored to 'n' is used in the enclosing expression, the value is never actually read from 'n'
if ((n = read_hci_event(fd, resp, 4)) < 0) {
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/hciattach.c:930:7: warning: Although the value stored to 'n' is used in the enclosing expression, the value is never actually read from 'n'
if ((n = read_hci_event(fd, resp, 4)) < 0) {
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/hciattach.c:974:7: warning: Although the value stored to 'n' is used in the enclosing expression, the value is never actually read from 'n'
if ((n = read_hci_event(fd, resp, 6)) < 0) {
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
6 warnings generated.
src/shared/bap.c:1531:8: warning: Use of memory after it is freed
bap = bt_bap_ref_safe(bap);
^~~~~~~~~~~~~~~~~~~~
src/shared/bap.c:2312:20: warning: Use of memory after it is freed
return queue_find(stream->bap->streams, NULL, stream);
^~~~~~~~~~~~~~~~~~~~
2 warnings generated.
src/oui.c:50:2: warning: Value stored to 'hwdb' is never read
hwdb = udev_hwdb_unref(hwdb);
^ ~~~~~~~~~~~~~~~~~~~~~
src/oui.c:53:2: warning: Value stored to 'udev' is never read
udev = udev_unref(udev);
^ ~~~~~~~~~~~~~~~~
2 warnings generated.
tools/rfcomm.c:234:3: warning: Value stored to 'i' is never read
i = execvp(cmdargv[0], cmdargv);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/rfcomm.c:234:7: warning: Null pointer passed to 1st parameter expecting 'nonnull'
i = execvp(cmdargv[0], cmdargv);
^~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/rfcomm.c:354:8: warning: Although the value stored to 'fd' is used in the enclosing expression, the value is never actually read from 'fd'
if ((fd = open(devname, O_RDONLY | O_NOCTTY)) < 0) {
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/rfcomm.c:497:14: warning: Assigned value is garbage or undefined
req.channel = raddr.rc_channel;
^ ~~~~~~~~~~~~~~~~
tools/rfcomm.c:515:8: warning: Although the value stored to 'fd' is used in the enclosing expression, the value is never actually read from 'fd'
if ((fd = open(devname, O_RDONLY | O_NOCTTY)) < 0) {
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
5 warnings generated.
tools/hcidump.c:180:9: warning: Potential leak of memory pointed to by 'dp'
if (fds[i].fd == sock)
^~~
tools/hcidump.c:248:17: warning: Assigned value is garbage or undefined
dh->ts_sec = htobl(frm.ts.tv_sec);
^ ~~~~~~~~~~~~~~~~~~~~
tools/hcidump.c:326:9: warning: 1st function call argument is an uninitialized value
if (be32toh(dp.flags) & 0x02) {
^~~~~~~~~~~~~~~~~
/usr/include/endian.h:46:22: note: expanded from macro 'be32toh'
# define be32toh(x) __bswap_32 (x)
^~~~~~~~~~~~~~
tools/hcidump.c:341:20: warning: 1st function call argument is an uninitialized value
frm.data_len = be32toh(dp.len);
^~~~~~~~~~~~~~~
/usr/include/endian.h:46:22: note: expanded from macro 'be32toh'
# define be32toh(x) __bswap_32 (x)
^~~~~~~~~~~~~~
tools/hcidump.c:346:14: warning: 1st function call argument is an uninitialized value
opcode = be32toh(dp.flags) & 0xffff;
^~~~~~~~~~~~~~~~~
/usr/include/endian.h:46:22: note: expanded from macro 'be32toh'
# define be32toh(x) __bswap_32 (x)
^~~~~~~~~~~~~~
tools/hcidump.c:384:17: warning: Assigned value is garbage or undefined
frm.data_len = btohs(dh.len);
^ ~~~~~~~~~~~~~
tools/hcidump.c:394:11: warning: Assigned value is garbage or undefined
frm.len = frm.data_len;
^ ~~~~~~~~~~~~
tools/hcidump.c:398:9: warning: 1st function call argument is an uninitialized value
ts = be64toh(ph.ts);
^~~~~~~~~~~~~~
/usr/include/endian.h:51:22: note: expanded from macro 'be64toh'
# define be64toh(x) __bswap_64 (x)
^~~~~~~~~~~~~~
tools/hcidump.c:403:13: warning: 1st function call argument is an uninitialized value
frm.in = be32toh(dp.flags) & 0x01;
^~~~~~~~~~~~~~~~~
/usr/include/endian.h:46:22: note: expanded from macro 'be32toh'
# define be32toh(x) __bswap_32 (x)
^~~~~~~~~~~~~~
tools/hcidump.c:408:11: warning: Assigned value is garbage or undefined
frm.in = dh.in;
^ ~~~~~
tools/hcidump.c:437:7: warning: Null pointer passed to 1st parameter expecting 'nonnull'
fd = open(file, open_flags, 0644);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
11 warnings generated.
tools/ciptool.c:350:7: warning: 5th function call argument is an uninitialized value
sk = do_connect(ctl, dev_id, &src, &dst, psm, (1 << CMTP_LOOPBACK));
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
src/sdp-xml.c:126:10: warning: Assigned value is garbage or undefined
buf[1] = data[i + 1];
^ ~~~~~~~~~~~
src/sdp-xml.c:300:11: warning: Assigned value is garbage or undefined
buf[1] = data[i + 1];
^ ~~~~~~~~~~~
src/sdp-xml.c:338:11: warning: Assigned value is garbage or undefined
buf[1] = data[i + 1];
^ ~~~~~~~~~~~
3 warnings generated.
tools/sdptool.c:941:26: warning: Result of 'malloc' is converted to a pointer of type 'uint32_t', which is incompatible with sizeof operand type 'int'
uint32_t *value_int = malloc(sizeof(int));
~~~~~~~~~~ ^~~~~~ ~~~~~~~~~~~
tools/sdptool.c:980:4: warning: 1st function call argument is an uninitialized value
free(allocArray[i]);
^~~~~~~~~~~~~~~~~~~
tools/sdptool.c:3777:2: warning: Potential leak of memory pointed to by 'si.name'
return add_service(0, &si);
^~~~~~~~~~~~~~~~~~~~~~~~~~
tools/sdptool.c:4112:4: warning: Potential leak of memory pointed to by 'context.svc'
return -1;
^~~~~~~~~
4 warnings generated.
tools/avtest.c:243:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 3);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:253:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 4);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:262:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 3);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:276:5: warning: Value stored to 'len' is never read
len = write(sk, buf,
^ ~~~~~~~~~~~~~~
tools/avtest.c:283:5: warning: Value stored to 'len' is never read
len = write(sk, buf,
^ ~~~~~~~~~~~~~~
tools/avtest.c:290:5: warning: Value stored to 'len' is never read
len = write(sk, buf,
^ ~~~~~~~~~~~~~~
tools/avtest.c:297:5: warning: Value stored to 'len' is never read
len = write(sk, buf,
^ ~~~~~~~~~~~~~~
tools/avtest.c:309:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 4);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:313:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 2);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:322:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 3);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:326:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 2);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:335:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 3);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:342:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 2);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:364:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 4);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:368:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 2);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:377:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 3);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:381:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 2);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:394:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 4);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:398:5: warning: Value stored to 'len' is never read
len = write(sk, buf, 2);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:405:4: warning: Value stored to 'len' is never read
len = write(sk, buf, 2);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:415:4: warning: Value stored to 'len' is never read
len = write(sk, buf, 2);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:580:3: warning: Value stored to 'len' is never read
len = write(sk, buf, 2);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:588:3: warning: Value stored to 'len' is never read
len = write(sk, buf, invalid ? 2 : 3);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/avtest.c:602:3: warning: Value stored to 'len' is never read
len = write(sk, buf, 4 + media_transport_size);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/avtest.c:615:3: warning: Value stored to 'len' is never read
len = write(sk, buf, 3);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:625:3: warning: Value stored to 'len' is never read
len = write(sk, buf, 3);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:637:3: warning: Value stored to 'len' is never read
len = write(sk, buf, 3);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:652:3: warning: Value stored to 'len' is never read
len = write(sk, buf, 3);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:664:3: warning: Value stored to 'len' is never read
len = write(sk, buf, 3);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:673:3: warning: Value stored to 'len' is never read
len = write(sk, buf, 3);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:680:3: warning: Value stored to 'len' is never read
len = write(sk, buf, 2);
^ ~~~~~~~~~~~~~~~~~
tools/avtest.c:716:2: warning: Value stored to 'len' is never read
len = write(sk, buf, AVCTP_HEADER_LENGTH + sizeof(play_pressed));
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
32 warnings generated.
tools/btproxy.c:836:15: warning: Null pointer passed to 1st parameter expecting 'nonnull'
tcp_port = atoi(optarg);
^~~~~~~~~~~~
tools/btproxy.c:839:8: warning: Null pointer passed to 1st parameter expecting 'nonnull'
if (strlen(optarg) > 3 && !strncmp(optarg, "hci", 3))
^~~~~~~~~~~~~~
2 warnings generated.
tools/iso-tester.c:1902:7: warning: Potential leak of memory pointed to by 'addr'
err = bind(sk, (struct sockaddr *) addr, sizeof(*addr) +
~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/iso-tester.c:1910:7: warning: Potential leak of memory pointed to by 'addr'
err = bind(sk, (struct sockaddr *) addr, sizeof(*addr));
~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 warnings generated.
tools/create-image.c:76:3: warning: Value stored to 'fd' is never read
fd = -1;
^ ~~
tools/create-image.c:84:3: warning: Value stored to 'fd' is never read
fd = -1;
^ ~~
tools/create-image.c:92:3: warning: Value stored to 'fd' is never read
fd = -1;
^ ~~
tools/create-image.c:105:2: warning: Value stored to 'fd' is never read
fd = -1;
^ ~~
4 warnings generated.
tools/check-selftest.c:42:3: warning: Value stored to 'ptr' is never read
ptr = fgets(result, sizeof(result), fp);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
tools/btgatt-server.c:1212:2: warning: Value stored to 'argv' is never read
argv -= optind;
^ ~~~~~~
1 warning generated.
tools/btgatt-client.c:1824:2: warning: Value stored to 'argv' is never read
argv += optind;
^ ~~~~~~
1 warning generated.
tools/gatt-service.c:294:2: warning: 2nd function call argument is an uninitialized value
chr_write(chr, value, len);
^~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
tools/obex-server-tool.c:133:13: warning: Null pointer passed to 1st parameter expecting 'nonnull'
data->fd = open(name, O_WRONLY | O_CREAT | O_NOCTTY, 0600);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/obex-server-tool.c:192:13: warning: Null pointer passed to 1st parameter expecting 'nonnull'
data->fd = open(name, O_RDONLY | O_NOCTTY, 0);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 warnings generated.
tools/btpclientctl.c:402:3: warning: Value stored to 'bit' is never read
bit = 0;
^ ~
tools/btpclientctl.c:1655:2: warning: Null pointer passed to 2nd parameter expecting 'nonnull'
memcpy(cp->data, ad_data, ad_len);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 warnings generated.
src/sdpd-request.c:211:13: warning: Result of 'malloc' is converted to a pointer of type 'char', which is incompatible with sizeof operand type 'uint16_t'
pElem = malloc(sizeof(uint16_t));
^~~~~~ ~~~~~~~~~~~~~~~~
src/sdpd-request.c:239:13: warning: Result of 'malloc' is converted to a pointer of type 'char', which is incompatible with sizeof operand type 'uint32_t'
pElem = malloc(sizeof(uint32_t));
^~~~~~ ~~~~~~~~~~~~~~~~
2 warnings generated.
android/avrcp-lib.c:1968:3: warning: 1st function call argument is an uninitialized value
g_free(text[i]);
^~~~~~~~~~~~~~~
1 warning generated.
profiles/audio/avdtp.c:893:25: warning: Use of memory after it is freed
session->prio_queue = g_slist_remove(session->prio_queue, req);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
profiles/audio/avdtp.c:900:24: warning: Use of memory after it is freed
session->req_queue = g_slist_remove(session->req_queue, req);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 warnings generated.
profiles/audio/a2dp.c:375:8: warning: Use of memory after it is freed
if (!cb->resume_cb)
^~~~~~~~~~~~~
profiles/audio/a2dp.c:3269:20: warning: Access to field 'starting' results in a dereference of a null pointer (loaded from variable 'stream')
stream->starting = TRUE;
~~~~~~ ^
profiles/audio/a2dp.c:3272:8: warning: Access to field 'suspending' results in a dereference of a null pointer (loaded from variable 'stream')
if (!stream->suspending && stream->suspend_timer) {
^~~~~~~~~~~~~~~~~~
profiles/audio/a2dp.c:3332:22: warning: Access to field 'suspending' results in a dereference of a null pointer (loaded from variable 'stream')
stream->suspending = TRUE;
~~~~~~ ^
4 warnings generated.
profiles/health/hdp.c:644:3: warning: Use of memory after it is freed
hdp_tmp_dc_data_unref(dc_data);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
profiles/health/hdp.c:800:19: warning: Use of memory after it is freed
path = g_strdup(chan->path);
^~~~~~~~~~
profiles/health/hdp.c:1779:6: warning: Use of memory after it is freed
hdp_tmp_dc_data_ref(hdp_conn),
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
profiles/health/hdp.c:1836:30: warning: Use of memory after it is freed
reply = g_dbus_create_error(data->msg, ERROR_INTERFACE ".HealthError",
^~~~~~~~~
4 warnings generated.
profiles/audio/avrcp.c:1961:2: warning: Value stored to 'operands' is never read
operands += sizeof(*pdu);
^ ~~~~~~~~~~~~
1 warning generated.
profiles/health/hdp_util.c:1052:2: warning: Use of memory after it is freed
conn_data->func(conn_data->data, gerr);
^~~~~~~~~~~~~~~
1 warning generated.
attrib/gatt.c:970:2: warning: Potential leak of memory pointed to by 'long_write'
return prepare_write(long_write);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
profiles/audio/bap.c:1024:18: warning: Access to field 'data' results in a dereference of a null pointer (loaded from field 'ep')
bap_update_cigs(setup->ep->data);
^~~~~~~~~~~~~~~
1 warning generated.
src/sdpd-request.c:211:13: warning: Result of 'malloc' is converted to a pointer of type 'char', which is incompatible with sizeof operand type 'uint16_t'
pElem = malloc(sizeof(uint16_t));
^~~~~~ ~~~~~~~~~~~~~~~~
src/sdpd-request.c:239:13: warning: Result of 'malloc' is converted to a pointer of type 'char', which is incompatible with sizeof operand type 'uint32_t'
pElem = malloc(sizeof(uint32_t));
^~~~~~ ~~~~~~~~~~~~~~~~
2 warnings generated.
src/sdp-client.c:353:14: warning: Access to field 'cb' results in a dereference of a null pointer
(*ctxt)->cb = cb;
~~~~~~~~~~~~^~~~
1 warning generated.
src/sdp-xml.c:126:10: warning: Assigned value is garbage or undefined
buf[1] = data[i + 1];
^ ~~~~~~~~~~~
src/sdp-xml.c:300:11: warning: Assigned value is garbage or undefined
buf[1] = data[i + 1];
^ ~~~~~~~~~~~
src/sdp-xml.c:338:11: warning: Assigned value is garbage or undefined
buf[1] = data[i + 1];
^ ~~~~~~~~~~~
3 warnings generated.
src/gatt-database.c:1173:10: warning: Value stored to 'bits' during its initialization is never read
uint8_t bits[] = { BT_GATT_CHRC_CLI_FEAT_ROBUST_CACHING,
^~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
gobex/gobex-header.c:95:2: warning: Null pointer passed to 2nd parameter expecting 'nonnull'
memcpy(to, from, count);
^~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
gobex/gobex-transfer.c:423:7: warning: Use of memory after it is freed
if (!g_slist_find(transfers, transfer))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
obexd/client/pbap.c: In function ‘pbap_init’:
obexd/client/pbap.c:1530:1: error: control reaches end of non-void function [-Werror=return-type]
1530 | }
| ^
cc1: all warnings being treated as errors
make[1]: *** [Makefile:9930: obexd/client/obexd-pbap.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:4690: all] Error 2
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH BlueZ RFC] Refactor uid_state handling
2025-06-20 11:31 [PATCH BlueZ RFC] Refactor uid_state handling Andrew Sayers
2025-06-20 12:54 ` [BlueZ,RFC] " bluez.test.bot
@ 2025-06-20 13:59 ` Luiz Augusto von Dentz
1 sibling, 0 replies; 3+ messages in thread
From: Luiz Augusto von Dentz @ 2025-06-20 13:59 UTC (permalink / raw)
To: Andrew Sayers; +Cc: linux-bluetooth
Hi Andrew,
On Fri, Jun 20, 2025 at 7:39 AM Andrew Sayers
<kernel.org@pileofstuff.org> wrote:
>
> We talked recently about refactoring logind.{c,h} to fit more neatly
> into the program's wider architecture. This patch sketches out a
> possible approach. It compiles, but has not been tested beyond that.
> If I'm on the right track, I'll come back with a proper solution.
>
> The old API provided callbacks on high-level "init" and "exit" events.
> That made sense for the limited case it was used for, but seems
> unlikely to scale if this starts picking up more use cases. So the
> RFC API just provides a single event that passes a `logind_cb_context`
> struct to callbacks, and provides `LOGIND_USER_IS_ACTIVE(ctxt)` to
> replicate the old behaviour.
>
> The old API invited individual transports/drivers to use it directly,
> which again made sense for version 1, but bypassed `transport.h` and
> `driver.h`. The RFC API registers callbacks with `driver.h` and
> `transport.h` in a more normal way, and they pass `logind_cb_context`
> straight through to the callbacks. That means we won't need to do
> much refactoring if the struct changes some day, but also means
> individual transports/drivers need to know about `logind.h`.
> The alternative would look something like this:
>
> --- obexd/client/driver.h
> +++ obexd/client/driver.h
> struct obc_driver {
> const char *service;
> const char *uuid;
> void *target;
> gsize target_len;
> void *(*supported_features) (struct obc_session *session);
> int (*probe) (struct obc_session *session);
> void (*remove) (struct obc_session *session);
> + int (*uid_state) (const char *state, const int seats);
Don't think we really need to expose the seat state, etc, instead I
would just have a seated and unseated callback which just indicates to
the driver it can do the likes of RegisterProfile/UnregisterProfile
that way we can restrict the handling of seats just to driver.c and I
don't think we really need it for transport drivers the transports are
usable if there are drivers in obc_session_create:
driver = obc_driver_find(service);
if (driver == NULL)
return NULL;
So once we integrate the seat logic into the driver.c I assume
obc_driver_find will only return drivers that are seated, so
users/applications on a unseated D-Bus connection won't be able to
create any sessions.
> };
> --- obexd/client/driver.c
> +++ obexd/client/driver.c
> +static void call_cb(gpointer data, gpointer ctxt_)
> +{
> + struct obc_driver *driver = (struct obc_driver *)data;
> + struct logind_cb_context *ctxt = (struct logind_cb_context *)ctxt_;
> + if (driver->uid_state) {
> + ctxt->res |= driver->uid_state(ctxt->state, ctxt->seats);
> + }
> +}
>
> That would have a better best case (fewer `#include logind.h`s strewn
> throughout the code) but a worse worst case (bigger refactoring job if
> we need to pass an extra argument some day). I think the RFC patch is
> a better balance of risks, but am happy either way.
>
> The RFC patches for `pbap.c` and `bluetooth.c` are designed to show
> the difference between the old and new API. A proper patch will
> probably get rid of the `_cb()`, `_init()` and `_exit()` functions.
>
> Suggested-by: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
> Signed-off-by: Andrew Sayers <kernel.org@pileofstuff.org>
> ---
> obexd/client/driver.c | 18 +++++
> obexd/client/driver.h | 5 ++
> obexd/client/pbap.c | 62 ++++++++++------
> obexd/plugins/bluetooth.c | 30 ++++++--
> obexd/src/logind.c | 152 ++++++++++++++++++--------------------
> obexd/src/logind.h | 59 ++++++++++++---
> obexd/src/main.c | 12 +++
> obexd/src/transport.c | 17 +++++
> obexd/src/transport.h | 5 ++
> 9 files changed, 237 insertions(+), 123 deletions(-)
>
> diff --git a/obexd/client/driver.c b/obexd/client/driver.c
> index 195cdd2f1..d5d0fe2ab 100644
> --- a/obexd/client/driver.c
> +++ b/obexd/client/driver.c
> @@ -74,3 +74,21 @@ void obc_driver_unregister(struct obc_driver *driver)
>
> drivers = g_slist_remove(drivers, driver);
> }
> +
> +static void call_cb(gpointer data, gpointer ctxt)
> +{
> + struct obc_driver *driver = (struct obc_driver *)data;
> +
> + if (driver->uid_state)
> + driver->uid_state(((struct logind_cb_context *)ctxt));
> +}
> +
> +static void call_uid_state_cb(gpointer ctxt)
> +{
> + g_slist_foreach(drivers, call_cb, ctxt);
> +}
> +
> +gboolean obc_driver_init(void)
> +{
> + return logind_register(call_uid_state_cb) >= 0;
> +}
> diff --git a/obexd/client/driver.h b/obexd/client/driver.h
> index cc4cace7b..928c0c558 100644
> --- a/obexd/client/driver.h
> +++ b/obexd/client/driver.h
> @@ -8,6 +8,9 @@
> *
> */
>
> +#include "obexd/src/logind.h"
> +struct obc_session;
> +
> struct obc_driver {
> const char *service;
> const char *uuid;
> @@ -16,8 +19,10 @@ struct obc_driver {
> void *(*supported_features) (struct obc_session *session);
> int (*probe) (struct obc_session *session);
> void (*remove) (struct obc_session *session);
> + void (*uid_state) (struct logind_cb_context *ctxt);
> };
>
> int obc_driver_register(struct obc_driver *driver);
> void obc_driver_unregister(struct obc_driver *driver);
> struct obc_driver *obc_driver_find(const char *pattern);
> +gboolean obc_driver_init(void);
> diff --git a/obexd/client/pbap.c b/obexd/client/pbap.c
> index 78c46bf86..dc2a519e5 100644
> --- a/obexd/client/pbap.c
> +++ b/obexd/client/pbap.c
> @@ -146,6 +146,9 @@ static DBusConnection *system_conn;
> static unsigned int listener_id;
> static char *client_path;
>
> +static int pbap_init_cb(void);
> +static void pbap_exit_cb(void);
> +
> static struct pending_request *pending_request_new(struct pbap_data *pbap,
> DBusMessage *message)
> {
> @@ -1300,6 +1303,20 @@ static void pbap_remove(struct obc_session *session)
> g_dbus_unregister_interface(conn, path, PBAP_INTERFACE);
> }
>
> +static gboolean user_was_active = FALSE;
> +static void pbap_uid_state(struct logind_cb_context *ctxt)
> +{
> + gboolean user_is_active = LOGIND_USER_IS_ACTIVE(ctxt);
> +
> + if (user_was_active == user_is_active)
> + return;
> + user_was_active = user_is_active;
> + if (user_is_active)
> + pbap_init_cb();
> + else
> + pbap_exit_cb();
> +}
> +
> static DBusMessage *pbap_release(DBusConnection *conn,
> DBusMessage *msg, void *data)
> {
> @@ -1452,13 +1469,13 @@ static struct obc_driver pbap = {
> .target_len = OBEX_PBAP_UUID_LEN,
> .supported_features = pbap_supported_features,
> .probe = pbap_probe,
> - .remove = pbap_remove
> + .remove = pbap_remove,
> + .uid_state = pbap_uid_state
> };
>
> -static int pbap_init_cb(gboolean at_register)
> +static int pbap_init_cb(void)
> {
> int err;
> - (void)at_register;
>
> DBG("");
>
> @@ -1483,7 +1500,7 @@ static int pbap_init_cb(gboolean at_register)
> return 0;
> }
>
> -static void pbap_exit_cb(gboolean at_unregister)
> +static void pbap_exit_cb(void)
> {
> DBusMessage *msg;
> DBusMessageIter iter;
> @@ -1491,22 +1508,28 @@ static void pbap_exit_cb(gboolean at_unregister)
>
> DBG("");
>
> - if (!at_unregister) {
> - client_path = g_strconcat("/org/bluez/obex/", uuid, NULL);
> - g_strdelimit(client_path, "-", '_');
> + client_path = g_strconcat("/org/bluez/obex/", uuid, NULL);
> + g_strdelimit(client_path, "-", '_');
>
> - msg = dbus_message_new_method_call("org.bluez", "/org/bluez",
> - "org.bluez.ProfileManager1",
> - "UnregisterProfile");
> + msg = dbus_message_new_method_call("org.bluez", "/org/bluez",
> + "org.bluez.ProfileManager1",
> + "UnregisterProfile");
>
> - dbus_message_iter_init_append(msg, &iter);
> + dbus_message_iter_init_append(msg, &iter);
>
> - dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH,
> - &client_path);
> + dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH,
> + &client_path);
>
> - g_dbus_send_message(system_conn, msg);
> - }
> + g_dbus_send_message(system_conn, msg);
>
> + pbap_exit();
> +}
> +
> +int pbap_init(void)
> +{
> +}
> +void pbap_exit(void)
> +{
> g_dbus_remove_watch(system_conn, listener_id);
>
> unregister_profile();
> @@ -1524,12 +1547,3 @@ static void pbap_exit_cb(gboolean at_unregister)
>
> obc_driver_unregister(&pbap);
> }
> -
> -int pbap_init(void)
> -{
> - return logind_register(pbap_init_cb, pbap_exit_cb);
> -}
> -void pbap_exit(void)
> -{
> - return logind_unregister(pbap_init_cb, pbap_exit_cb);
> -}
> diff --git a/obexd/plugins/bluetooth.c b/obexd/plugins/bluetooth.c
> index 355921479..ebff88db3 100644
> --- a/obexd/plugins/bluetooth.c
> +++ b/obexd/plugins/bluetooth.c
> @@ -57,6 +57,9 @@ static DBusMessage *profile_release(DBusConnection *conn, DBusMessage *msg,
> return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
> }
>
> +static int bluetooth_init_cb(void);
> +static void bluetooth_exit_cb(void);
> +
> static void connect_event(GIOChannel *io, GError *err, void *user_data)
> {
> int sk = g_io_channel_unix_get_fd(io);
> @@ -381,6 +384,20 @@ static void bluetooth_stop(void *data)
> profiles = NULL;
> }
>
> +static gboolean user_was_active = FALSE;
> +static void bluetooth_uid_state(struct logind_cb_context *ctxt)
> +{
> + gboolean user_is_active = LOGIND_USER_IS_ACTIVE(ctxt);
> +
> + if (user_was_active == user_is_active)
> + return;
> + user_was_active = user_is_active;
> + if (user_is_active)
> + bluetooth_init_cb();
> + else
> + bluetooth_exit_cb();
> +}
> +
> static int bluetooth_getpeername(GIOChannel *io, char **name)
> {
> GError *gerr = NULL;
> @@ -422,14 +439,14 @@ static const struct obex_transport_driver driver = {
> .start = bluetooth_start,
> .getpeername = bluetooth_getpeername,
> .getsockname = bluetooth_getsockname,
> - .stop = bluetooth_stop
> + .stop = bluetooth_stop,
> + .uid_state = bluetooth_uid_state
> };
>
> static unsigned int listener_id = 0;
>
> -static int bluetooth_init_cb(gboolean at_register)
> +static int bluetooth_init_cb(void)
> {
> - (void)at_register;
> connection = g_dbus_setup_private(DBUS_BUS_SYSTEM, NULL, NULL);
> if (connection == NULL)
> return -EPERM;
> @@ -440,10 +457,9 @@ static int bluetooth_init_cb(gboolean at_register)
> return obex_transport_driver_register(&driver);
> }
>
> -static void bluetooth_exit_cb(gboolean at_unregister)
> +static void bluetooth_exit_cb(void)
> {
> GSList *l;
> - (void)at_unregister;
>
> g_dbus_remove_watch(connection, listener_id);
>
> @@ -467,11 +483,11 @@ static void bluetooth_exit_cb(gboolean at_unregister)
>
> static int bluetooth_init(void)
> {
> - return logind_register(bluetooth_init_cb, bluetooth_exit_cb);
> }
> static void bluetooth_exit(void)
> {
> - return logind_unregister(bluetooth_init_cb, bluetooth_exit_cb);
> + if (user_was_active)
> + bluetooth_exit_cb();
> }
>
> OBEX_PLUGIN_DEFINE(bluetooth, bluetooth_init, bluetooth_exit)
> diff --git a/obexd/src/logind.c b/obexd/src/logind.c
> index b4279b209..a8ea25543 100644
> --- a/obexd/src/logind.c
> +++ b/obexd/src/logind.c
> @@ -27,66 +27,48 @@
>
> static sd_login_monitor * monitor;
> static int uid;
> -static gboolean active = FALSE;
> static gboolean monitoring_enabled = TRUE;
> static guint event_source;
> static guint timeout_source;
>
> -struct callback_pair {
> - logind_init_cb init_cb;
> - logind_exit_cb exit_cb;
> -};
> -
> GSList *callbacks;
>
> -static void call_init_cb(gpointer data, gpointer user_data)
> -{
> - int res;
> -
> - res = ((struct callback_pair *)data)->init_cb(FALSE);
> - if (res)
> - *(int *)user_data = res;
> -}
> -static void call_exit_cb(gpointer data, gpointer user_data)
> +static void call_cb(gpointer data, gpointer user_data)
> {
> - ((struct callback_pair *)data)->exit_cb(FALSE);
> + (*((logind_cb *)data))(user_data);
> }
>
> -static int update(void)
> +static void logind_cb_context_init(struct logind_cb_context *ctxt)
> {
> - char *state = NULL;
> - gboolean state_is_active;
> - int res;
> -
> - res = sd_login_monitor_flush(monitor);
> - if (res < 0)
> - return res;
> - res = sd_uid_get_state(uid, &state);
> - state_is_active = g_strcmp0(state, "active");
> - free(state);
> - if (res < 0)
> - return res;
> -
> - if (state_is_active) {
> - if (!active)
> - return 0;
> - } else {
> - res = sd_uid_get_seats(uid, 1, NULL);
> - if (res < 0)
> - return res;
> - if (active == !!res)
> - return 0;
> + ctxt->res = sd_login_monitor_flush(monitor);
> + if (ctxt->res < 0)
> + return;
> +
> + ctxt->res = ctxt->seats = sd_uid_get_seats(uid, 1, NULL);
> + if (ctxt->res < 0)
> + return;
> +
> + /*
> + * the documentation for sd_uid_get_state() isn't clear about
> + * what to do with the state on error. The following should
> + * be safe even if the behaviour changes in future
> + */
> + ctxt->state = 0;
> + ctxt->res = sd_uid_get_state(uid, (char **)&ctxt->state);
> + if (ctxt->res <= 0) {
> + free((char *)ctxt->state);
> + return;
> }
> - active ^= TRUE;
> - res = 0;
> - g_slist_foreach(callbacks, active ? call_init_cb : call_exit_cb, &res);
> - return res;
> +
> + ctxt->res = 0;
> + return;
> }
>
> static gboolean timeout_handler(gpointer user_data);
>
> static int check_event(void)
> {
> + struct logind_cb_context ctxt;
> uint64_t timeout_usec;
> int res;
>
> @@ -95,9 +77,14 @@ static int check_event(void)
> return res;
> if (!monitoring_enabled)
> return 0;
> - res = update();
> - if (res < 0)
> - return res;
> +
> + logind_cb_context_init(&ctxt);
> + if (ctxt.res)
> + return ctxt.res;
> + g_slist_foreach(callbacks, call_cb, &ctxt);
> + free((char *)ctxt.state);
> + if (ctxt.res)
> + return ctxt.res;
>
> res = sd_login_monitor_get_timeout(monitor, &timeout_usec);
> if (res < 0)
> @@ -154,6 +141,7 @@ static gboolean timeout_handler(gpointer user_data)
>
> static int logind_init(void)
> {
> + struct logind_cb_context ctxt;
> GIOChannel *channel;
> int events;
> int fd;
> @@ -174,11 +162,6 @@ static int logind_init(void)
> goto FAIL;
> }
>
> - // Check this after creating the monitor, in case of race conditions:
> - res = update();
> - if (res < 0)
> - goto FAIL;
> -
> events = res = sd_login_monitor_get_events(monitor);
> if (res < 0)
> goto FAIL;
> @@ -202,7 +185,10 @@ static int logind_init(void)
> FAIL:
> sd_login_monitor_unref(monitor);
> monitoring_enabled = FALSE;
> - active = TRUE;
> + ctxt.state = "active";
> + ctxt.seats = 1;
> + ctxt.res = 0;
> + g_slist_foreach(callbacks, call_cb, &ctxt);
> return res;
> }
>
> @@ -219,17 +205,18 @@ static void logind_exit(void)
> sd_login_monitor_unref(monitor);
> }
>
> -static gint find_cb(gconstpointer a, gconstpointer b)
> +int logind_register(logind_cb cb)
> {
> - return ((struct callback_pair *)a)->init_cb - (logind_init_cb)b;
> -}
> + struct logind_cb_context ctxt;
>
> -int logind_register(logind_init_cb init_cb, logind_exit_cb exit_cb)
> -{
> - struct callback_pair *cbs;
> + logind_cb_context_init(&ctxt);
> + if (ctxt.res) {
> + free((char *)ctxt.state);
> + return ctxt.res;
> + }
>
> if (!monitoring_enabled)
> - return init_cb(TRUE);
> + goto CALL_CB;
> if (callbacks == NULL) {
> int res;
>
> @@ -237,24 +224,23 @@ int logind_register(logind_init_cb init_cb, logind_exit_cb exit_cb)
> if (res) {
> error("logind_init(): %s - login detection disabled",
> strerror(-res));
> - return init_cb(TRUE);
> + goto CALL_CB;
> }
> }
> - cbs = g_new(struct callback_pair, 1);
> - cbs->init_cb = init_cb;
> - cbs->exit_cb = exit_cb;
> - callbacks = g_slist_prepend(callbacks, cbs);
> - return active ? init_cb(TRUE) : 0;
> + callbacks = g_slist_prepend(callbacks, cb);
> +
> +CALL_CB:
> + cb(&ctxt);
> + free((char *)ctxt.state);
> + return ctxt.res;
> }
> -void logind_unregister(logind_init_cb init_cb, logind_exit_cb exit_cb)
> +void logind_unregister(logind_cb cb)
> {
> GSList *cb_node;
>
> if (!monitoring_enabled)
> - return exit_cb(TRUE);
> - if (active)
> - exit_cb(TRUE);
> - cb_node = g_slist_find_custom(callbacks, init_cb, find_cb);
> + return;
> + cb_node = g_slist_find(callbacks, cb);
> if (cb_node != NULL)
> callbacks = g_slist_delete_link(callbacks, cb_node);
> if (callbacks == NULL)
> @@ -263,20 +249,26 @@ void logind_unregister(logind_init_cb init_cb, logind_exit_cb exit_cb)
>
> int logind_set(gboolean enabled)
> {
> - int res = 0;
> -
> - if (monitoring_enabled == enabled)
> - return 0;
> -
> monitoring_enabled = enabled;
> if (enabled) {
> - active = FALSE;
> - return update();
> + struct logind_cb_context ctxt;
> +
> + logind_cb_context_init(&ctxt);
> + if (ctxt.res)
> + return ctxt.res;
> + g_slist_foreach(callbacks, call_cb, &ctxt);
> + free((char *)ctxt.state);
> + return ctxt.res;
> }
>
> - active = TRUE;
> - g_slist_foreach(callbacks, call_exit_cb, &res);
> - return res;
> + struct logind_cb_context ctxt = {
> + .state = "active",
> + .seats = 1,
> + .res = 0
> + };
> +
> + g_slist_foreach(callbacks, call_cb, &ctxt);
> + return ctxt.res;
> }
>
> #endif
> diff --git a/obexd/src/logind.h b/obexd/src/logind.h
> index 3cdb03338..243aa17bd 100644
> --- a/obexd/src/logind.h
> +++ b/obexd/src/logind.h
> @@ -8,30 +8,65 @@
> *
> */
>
> -typedef int (*logind_init_cb)(gboolean at_register);
> -typedef void (*logind_exit_cb)(gboolean at_unregister);
> +#ifndef OBEXD_SRC_LOGIND_H
> +#define OBEXD_SRC_LOGIND_H
> +
> +struct logind_cb_context {
> + const char *state;
> + int seats;
> + int res;
> +};
> +
> +typedef void (*logind_cb)(gpointer ctxt);
>
> #ifdef SYSTEMD
>
> -int logind_register(logind_init_cb init_cb, logind_exit_cb exit_cb);
> -void logind_unregister(logind_init_cb init_cb, logind_exit_cb exit_cb);
> +/*
> + * Register callback and call it with the current state
> + */
> +int logind_register(logind_cb init_cb);
> +/*
> + * Unregister callback but DO NOT call it -
> + * unregistration usually happens when the user is logging out,
> + * and other programs are going away.
> + *
> + * If possible, close resources at exit instead of at unregister.
> + * Otherwise, you will need to explicitly call your callback.
> + */
> +void logind_unregister(logind_cb cb);
> +/*
> + * Override the detected login state
> + */
> int logind_set(gboolean enabled);
>
> -#else
> +/* Recommended way to detect (in)activity */
> +#define LOGIND_USER_IS_ACTIVE(ctxt) \
> + (!g_strcmp0(ctxt->state, "active") && !!(ctxt->seats))
>
> -static inline int logind_register(logind_init_cb init_cb,
> - logind_exit_cb exit_cb)
> +#else /* SYSTEMD */
> +
> +static inline int logind_register(logind_cb cb)
> {
> - return init_cb(TRUE);
> + (void)cb;
> + struct logind_cb_context ctxt = {
> + .state = "active",
> + .seats = 1,
> + .res = 0
> + };
> + cb(&ctxt);
> + return ctxt.res;
> }
> -static inline void logind_unregister(logind_init_cb init_cb,
> - logind_exit_cb exit_cb)
> +static inline void logind_unregister(logind_cb cb)
> {
> - return exit_cb(TRUE);
> + (void)cb;
> }
> static inline int logind_set(gboolean enabled)
> {
> return 0;
> }
>
> -#endif
> +#define LOGIND_USER_IS_ACTIVE(...) 1
> +
> +#endif /* SYSTEMD */
> +
> +#endif /* OBEXD_SRC_LOGIND_H */
> diff --git a/obexd/src/main.c b/obexd/src/main.c
> index 6837f0d73..116888a78 100644
> --- a/obexd/src/main.c
> +++ b/obexd/src/main.c
> @@ -34,6 +34,9 @@
>
> #include "../client/manager.h"
>
> +#include "../client/driver.h"
> +#include "transport.h"
> +
> #include "log.h"
> #include "logind.h"
> #include "obexd.h"
> @@ -279,6 +282,15 @@ int main(int argc, char *argv[])
> if (option_system_bus)
> logind_set(FALSE);
>
> + if (obc_driver_init() == FALSE) {
> + error("manager_init failed");
> + exit(EXIT_FAILURE);
> + }
> + if (obex_transport_driver_init() == FALSE) {
> + error("manager_init failed");
> + exit(EXIT_FAILURE);
> + }
> +
> DBG("Entering main loop");
>
> main_loop = g_main_loop_new(NULL, FALSE);
> diff --git a/obexd/src/transport.c b/obexd/src/transport.c
> index 527d9ffce..fdd49b7a2 100644
> --- a/obexd/src/transport.c
> +++ b/obexd/src/transport.c
> @@ -79,3 +79,20 @@ obex_transport_driver_unregister(const struct obex_transport_driver *driver)
>
> drivers = g_slist_remove(drivers, driver);
> }
> +
> +static void call_cb(gpointer data, gpointer ctxt)
> +{
> + struct obex_transport_driver *driver =
> + (struct obex_transport_driver *)data;
> + if (driver->uid_state)
> + driver->uid_state(((struct logind_cb_context *)ctxt));
> +}
> +
> +static int call_uid_state_cb(gpointer ctxt)
> +{
> + g_slist_foreach(drivers, call_cb, ctxt);
> +}
> +
> +gboolean obex_transport_driver_init(void)
> +{
> +}
> diff --git a/obexd/src/transport.h b/obexd/src/transport.h
> index 322d8f526..087d8c211 100644
> --- a/obexd/src/transport.h
> +++ b/obexd/src/transport.h
> @@ -8,6 +8,9 @@
> *
> */
>
> +#include "obexd/src/logind.h"
> +struct obex_server;
> +
> struct obex_transport_driver {
> const char *name;
> uint16_t service;
> @@ -15,9 +18,11 @@ struct obex_transport_driver {
> int (*getpeername) (GIOChannel *io, char **name);
> int (*getsockname) (GIOChannel *io, char **name);
> void (*stop) (void *data);
> + void (*uid_state) (struct logind_cb_context *ctxt);
> };
>
> int obex_transport_driver_register(const struct obex_transport_driver *driver);
> void
> obex_transport_driver_unregister(const struct obex_transport_driver *driver);
> const GSList *obex_transport_driver_list(void);
> +gboolean obex_transport_driver_init(void);
> --
> 2.49.0
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-06-20 13:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-20 11:31 [PATCH BlueZ RFC] Refactor uid_state handling Andrew Sayers
2025-06-20 12:54 ` [BlueZ,RFC] " bluez.test.bot
2025-06-20 13:59 ` [PATCH BlueZ RFC] " Luiz Augusto von Dentz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox