Re: [PATCH BlueZ 11/12] agent: move defines and parsing for I/O capability to shared/mgnt

[Christian Eggers <ceggers@arri.de>]

Hi Luiz,

On Wednesday, 25 February 2026, 19:42:26 CET, Luiz Augusto von Dentz wrote:
> Hi Christian,
> 
> On Wed, Feb 25, 2026 at 1:38 PM Christian Eggers <ceggers@arri.de> wrote:
> >
> > Hi Luiz,
> >
> > On Wednesday, 25 February 2026, 17:38:49 CET, Luiz Augusto von Dentz wrote:
> > > Hi Christian,
> > >
> > > On Wed, Feb 25, 2026 at 11:22 AM Christian Eggers <ceggers@arri.de> wrote:
> > > >
> > > > Allow reusing in client/mgmt.c (next commit)
> > > > ---
> > > >  src/adapter.c     |  6 +++---
> > > >  src/agent.c       | 24 ++++--------------------
> > > >  src/agent.h       |  7 -------
> > > >  src/device.c      |  7 ++++---
> > >
> > > Shared changed should be sent separately since its license (LGPL) is
> > > different than the daemon (GPL).
> >
> > done
> >
> > >
> > > >  src/shared/mgmt.c | 32 ++++++++++++++++++++++++++++++++
> > > >  src/shared/mgmt.h | 10 ++++++++++
> > > >  6 files changed, 53 insertions(+), 33 deletions(-)
> > > >
> > > > diff --git a/src/adapter.c b/src/adapter.c
> > > > index 9bb1950a9f7d..bfabdf9a62ef 100644
> > > > --- a/src/adapter.c
> > > > +++ b/src/adapter.c
> > > > @@ -9156,7 +9156,7 @@ int adapter_set_io_capability(struct btd_adapter *adapter, uint8_t io_cap)
> > > >         struct mgmt_cp_set_io_capability cp;
> > > >
> > > >         if (!btd_opts.pairable) {
> > > > -               if (io_cap == IO_CAPABILITY_INVALID) {
> > > > +               if (io_cap == MGMT_IO_CAPABILITY_INVALID) {
> > > >                         if (adapter->current_settings & MGMT_SETTING_BONDABLE)
> > > >                                 set_mode(adapter, MGMT_OP_SET_BONDABLE, 0x00);
> > > >
> > > > @@ -9165,8 +9165,8 @@ int adapter_set_io_capability(struct btd_adapter *adapter, uint8_t io_cap)
> > > >
> > > >                 if (!(adapter->current_settings & MGMT_SETTING_BONDABLE))
> > > >                         set_mode(adapter, MGMT_OP_SET_BONDABLE, 0x01);
> > > > -       } else if (io_cap == IO_CAPABILITY_INVALID)
> > > > -               io_cap = IO_CAPABILITY_NOINPUTNOOUTPUT;
> > > > +       } else if (io_cap == MGMT_IO_CAPABILITY_INVALID)
> > > > +               io_cap = MGMT_IO_CAPABILITY_NOINPUTNOOUTPUT;
> > > >
> > > >         memset(&cp, 0, sizeof(cp));
> > > >         cp.io_capability = io_cap;
> > > > diff --git a/src/agent.c b/src/agent.c
> > > > index 3696575b83e6..794f81ebf18a 100644
> > > > --- a/src/agent.c
> > > > +++ b/src/agent.c
> > > > @@ -35,6 +35,7 @@
> > > >  #include "adapter.h"
> > > >  #include "device.h"
> > > >  #include "agent.h"
> > > > +#include "shared/mgmt.h"
> > > >  #include "shared/queue.h"
> > > >
> > > >  #define REQUEST_TIMEOUT (60 * 1000)            /* 60 seconds */
> > > > @@ -131,7 +132,7 @@ static void set_io_cap(struct btd_adapter *adapter, gpointer user_data)
> > > >         if (agent)
> > > >                 io_cap = agent->capability;
> > > >         else
> > > > -               io_cap = IO_CAPABILITY_INVALID;
> > > > +               io_cap = MGMT_IO_CAPABILITY_INVALID;
> > > >
> > > >         adapter_set_io_capability(adapter, io_cap);
> > > >  }
> > > > @@ -944,23 +945,6 @@ static void agent_destroy(gpointer data)
> > > >         agent_unref(agent);
> > > >  }
> > > >
> > > > -static uint8_t parse_io_capability(const char *capability)
> > > > -{
> > > > -       if (g_str_equal(capability, ""))
> > > > -               return IO_CAPABILITY_KEYBOARDDISPLAY;
> > > > -       if (g_str_equal(capability, "DisplayOnly"))
> > > > -               return IO_CAPABILITY_DISPLAYONLY;
> > > > -       if (g_str_equal(capability, "DisplayYesNo"))
> > > > -               return IO_CAPABILITY_DISPLAYYESNO;
> > > > -       if (g_str_equal(capability, "KeyboardOnly"))
> > > > -               return IO_CAPABILITY_KEYBOARDONLY;
> > > > -       if (g_str_equal(capability, "NoInputNoOutput"))
> > > > -               return IO_CAPABILITY_NOINPUTNOOUTPUT;
> > > > -       if (g_str_equal(capability, "KeyboardDisplay"))
> > > > -               return IO_CAPABILITY_KEYBOARDDISPLAY;
> > > > -       return IO_CAPABILITY_INVALID;
> > > > -}
> > > > -
> > > >  static DBusMessage *register_agent(DBusConnection *conn,
> > > >                                         DBusMessage *msg, void *user_data)
> > > >  {
> > > > @@ -979,8 +963,8 @@ static DBusMessage *register_agent(DBusConnection *conn,
> > > >                                                 DBUS_TYPE_INVALID) == FALSE)
> > > >                 return btd_error_invalid_args(msg);
> > > >
> > > > -       cap = parse_io_capability(capability);
> > > > -       if (cap == IO_CAPABILITY_INVALID)
> > > > +       cap = mgmt_parse_io_capability(capability);
> > > > +       if (cap == MGMT_IO_CAPABILITY_INVALID)
> > > >                 return btd_error_invalid_args(msg);
> > > >
> > > >         agent = agent_create(sender, path, cap);
> > > > diff --git a/src/agent.h b/src/agent.h
> > > > index bd0502030fa0..03731756849c 100644
> > > > --- a/src/agent.h
> > > > +++ b/src/agent.h
> > > > @@ -9,13 +9,6 @@
> > > >   *
> > > >   */
> > > >
> > > > -#define IO_CAPABILITY_DISPLAYONLY      0x00
> > > > -#define IO_CAPABILITY_DISPLAYYESNO     0x01
> > > > -#define IO_CAPABILITY_KEYBOARDONLY     0x02
> > > > -#define IO_CAPABILITY_NOINPUTNOOUTPUT  0x03
> > > > -#define IO_CAPABILITY_KEYBOARDDISPLAY  0x04
> > > > -#define IO_CAPABILITY_INVALID          0xFF
> > > > -
> > > >  struct agent;
> > > >
> > > >  typedef void (*agent_cb) (struct agent *agent, DBusError *err,
> > > > diff --git a/src/device.c b/src/device.c
> > > > index 0ef7dcc244d2..f7a84b807878 100644
> > > > --- a/src/device.c
> > > > +++ b/src/device.c
> > > > @@ -43,6 +43,7 @@
> > > >  #include "src/shared/gatt-client.h"
> > > >  #include "src/shared/gatt-server.h"
> > > >  #include "src/shared/ad.h"
> > > > +#include "src/shared/mgmt.h"
> > > >  #include "src/shared/timeout.h"
> > > >  #include "btio/btio.h"
> > > >  #include "bluetooth/mgmt.h"
> > > > @@ -3375,7 +3376,7 @@ static DBusMessage *pair_device(DBusConnection *conn, DBusMessage *msg,
> > > >         if (agent)
> > > >                 io_cap = agent_get_io_capability(agent);
> > > >         else
> > > > -               io_cap = IO_CAPABILITY_NOINPUTNOOUTPUT;
> > > > +               io_cap = MGMT_IO_CAPABILITY_NOINPUTNOOUTPUT;
> > > >
> > > >         bonding = bonding_request_new(msg, device, bdaddr_type, agent);
> > > >
> > > > @@ -6544,7 +6545,7 @@ static void att_connect_cb(GIOChannel *io, GError *gerr, gpointer user_data)
> > > >         if (device->bonding->agent)
> > > >                 io_cap = agent_get_io_capability(device->bonding->agent);
> > > >         else
> > > > -               io_cap = IO_CAPABILITY_NOINPUTNOOUTPUT;
> > > > +               io_cap = MGMT_IO_CAPABILITY_NOINPUTNOOUTPUT;
> > > >
> > > >         err = adapter_create_bonding(device->adapter, &device->bdaddr,
> > > >                                         device->bdaddr_type, io_cap);
> > > > @@ -7452,7 +7453,7 @@ static gboolean device_bonding_retry(gpointer data)
> > > >         if (bonding->agent)
> > > >                 io_cap = agent_get_io_capability(bonding->agent);
> > > >         else
> > > > -               io_cap = IO_CAPABILITY_NOINPUTNOOUTPUT;
> > > > +               io_cap = MGMT_IO_CAPABILITY_NOINPUTNOOUTPUT;
> > > >
> > > >         err = adapter_bonding_attempt(adapter, &device->bdaddr,
> > > >                                 device->bdaddr_type, io_cap);
> > > > diff --git a/src/shared/mgmt.c b/src/shared/mgmt.c
> > > > index 6a7eb5798cb8..300abbae1c50 100644
> > > > --- a/src/shared/mgmt.c
> > > > +++ b/src/shared/mgmt.c
> > > > @@ -81,6 +81,20 @@ struct mgmt_tlv_list {
> > > >         uint16_t size;
> > > >  };
> > > >
> > > > +struct arg_table {
> > > > +       const char *name;
> > > > +       unsigned value;
> > > > +};
> > > > +
> > > > +static const struct arg_table iocap_arguments[] = {
> > > > +       { "DisplayOnly", MGMT_IO_CAPABILITY_DISPLAYONLY },
> > > > +       { "DisplayYesNo", MGMT_IO_CAPABILITY_DISPLAYYESNO },
> > > > +       { "KeyboardOnly", MGMT_IO_CAPABILITY_KEYBOARDONLY },
> > > > +       { "NoInputNoOutput", MGMT_IO_CAPABILITY_NOINPUTNOOUTPUT },
> > > > +       { "KeyboardDisplay", MGMT_IO_CAPABILITY_KEYBOARDDISPLAY },
> > > > +       { NULL, 0}
> > > > +};
> > > > +
> > > >  static void destroy_request(void *data)
> > > >  {
> > > >         struct mgmt_request *request = data;
> > > > @@ -1039,3 +1053,21 @@ uint16_t mgmt_get_mtu(struct mgmt *mgmt)
> > > >
> > > >         return mgmt->mtu;
> > > >  }
> > > > +
> > > > +uint8_t mgmt_parse_io_capability(const char *capability)
> > > > +{
> > > > +       const char *arg;
> > > > +       int index = 0;
> > > > +
> > > > +       if (!strcmp(capability, ""))
> > > > +               return MGMT_IO_CAPABILITY_KEYBOARDDISPLAY;
> > > > +
> > > > +       while ((arg = iocap_arguments[index].name)) {
> > > > +               if (!strncmp(arg, capability, strlen(capability)))
> > > > +                       return iocap_arguments[index].value;
> > > > +
> > > > +               index++;
> > > > +       }
> > > > +
> > > > +       return MGMT_IO_CAPABILITY_INVALID;
> > > > +}
> > > > diff --git a/src/shared/mgmt.h b/src/shared/mgmt.h
> > > > index 2629fbd59cf6..a4c30075f7b7 100644
> > > > --- a/src/shared/mgmt.h
> > > > +++ b/src/shared/mgmt.h
> > > > @@ -13,6 +13,14 @@
> > > >
> > > >  #define MGMT_VERSION(v, r) (((v) << 16) + (r))
> > > >
> > > > +#define MGMT_IO_CAPABILITY_DISPLAYONLY         0x00
> > > > +#define MGMT_IO_CAPABILITY_DISPLAYYESNO                0x01
> > > > +#define MGMT_IO_CAPABILITY_KEYBOARDONLY                0x02
> > > > +#define MGMT_IO_CAPABILITY_NOINPUTNOOUTPUT     0x03
> > > > +#define MGMT_IO_CAPABILITY_KEYBOARDDISPLAY     0x04
> > > > +#define MGMT_IO_CAPABILITY_INVALID             0xFF
> > >
> > > Perhaps this should be an enum to ensure things like switch statements
> > > do check for unhandled values.
> >
> > also done. But now using an enum in src/adapter.h requires also including
> > "src/shared/mgmt.h" in every file where src/adapter.h is used. I've made
> > all these changes, but maybe we should really think about include guards.
> 
> Not following why would adapter.h requires it? Is adapter.h returning the enum?

diff --git a/src/adapter.h b/src/adapter.h
index 7a7e5c8f9dfd..377c6a8789fb 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -207,15 +207,18 @@ int btd_adapter_passkey_reply(struct btd_adapter *adapter,
                                uint32_t passkey);
 
 int adapter_create_bonding(struct btd_adapter *adapter, const bdaddr_t *bdaddr,
-                                       uint8_t addr_type, uint8_t io_cap);
+                                               uint8_t addr_type,
+                                               enum mgmt_io_capability io_cap);
 
 int adapter_bonding_attempt(struct btd_adapter *adapter, const bdaddr_t *bdaddr,
-                                       uint8_t addr_type, uint8_t io_cap);
+                                               uint8_t addr_type,
+                                               enum mgmt_io_capability io_cap);
 
 int adapter_cancel_bonding(struct btd_adapter *adapter, const bdaddr_t *bdaddr,
                                                        uint8_t addr_type);
 
-int adapter_set_io_capability(struct btd_adapter *adapter, uint8_t io_cap);
+int adapter_set_io_capability(struct btd_adapter *adapter,
+                                               enum mgmt_io_capability io_cap);
 
 int btd_adapter_read_local_oob_data(struct btd_adapter *adapter);
 

Compiler output:
In file included from plugins/autopair.c:28:
./src/adapter.h:211:54: warning: ‘enum mgmt_io_capability’ declared inside parameter list will not be visible outside of this definition or declaration
  211 |                                                 enum mgmt_io_capability io_cap);
      |                                                      ^~~~~~~~~~~~~~~~~~
./src/adapter.h:215:54: warning: ‘enum mgmt_io_capability’ declared inside parameter list will not be visible outside of this definition or declaration
  215 |                                                 enum mgmt_io_capability io_cap);
      |                                                      ^~~~~~~~~~~~~~~~~~
./src/adapter.h:221:54: warning: ‘enum mgmt_io_capability’ declared inside parameter list will not be visible outside of this definition or declaration
  221 |                                                 enum mgmt_io_capability io_cap);
      |                                                      ^~~~~~~~~~~~~~~~~~


So, `enum mgmt_io_capability` is not used as a return value, but also using it
as function argument requires prior declaration.

AFAIK, forward declaration of C-style enum is not possible. So if 
`enum mgmt_io_capability` is declared in shared/mgmt.h, this header must be
included prior src/adapter.h

Currently, src/adapter.h is included by 56 files. In nearly all of them, I had
to add inclusion of src/shared/mgmt.h.

regards,
Christian
> 
> > Shall I resubmit the whole series or do you want to pick the fine commits
> > first?
> 
> Ive already applied the changes that I didn't have any comments, so
> please rebase and send as a new set.
> 
> > regards,
> > Christian
> >
> > >
> > > > +
> > > > +
> > > >  typedef void (*mgmt_destroy_func_t)(void *user_data);
> > > >
> > > >  struct mgmt;
> > > > @@ -89,3 +97,5 @@ bool mgmt_unregister_index(struct mgmt *mgmt, uint16_t index);
> > > >  bool mgmt_unregister_all(struct mgmt *mgmt);
> > > >
> > > >  uint16_t mgmt_get_mtu(struct mgmt *mgmt);
> > > > +
> > > > +uint8_t mgmt_parse_io_capability(const char *capability);
> > > > --
> > > > 2.51.0
> > > >
> > > >
> > >
> > >
> > >
> >
> >
> >
> >
> 
> 
>