Re: [PATCH BlueZ 2/2] input: fix HoG compilation w/o HID

[Thomas Perale <thomas.perale@mind.be>]

Hi Luiz,

Thanks for your review

> This doesn't look correct, the input.conf is used to configure both
> HID and HOG, so why do you think it is a good idea to ignore it if HID
> has been disabled? What we probably should do is to move the portion
> of input_init that are HOG specific, e.g. LEAutoSecurity, which is
> what is being used above.
> This part we will probably have to duplicate the parsing of
> UserspaceHID from input_init.
My reasoning was that since I didn't saw the 'input.conf' file being 
read in the hog_init I thought it was for a reason. While for the 
'LEAutoSecurity' option it's indeed probably better to encapsulate the 
functions only in the HOG file. For the 'UserspaceHID' option as you 
said, reading the option will need to be duplicated in both input_init 
and hog_init. If this is OK for you to read this option two times then I 
can send a v2 of my patch.

Thomas

On 4/30/25 5:20 PM, Luiz Augusto von Dentz wrote:
> Hi Thomas,
>
> On Wed, Apr 30, 2025 at 10:42 AM Thomas Perale <thomas.perale@mind.be> wrote:
>> Commit [1] introduced a dependency with the HID plugin in the HoG code
>> As a result, building with --disable-hid --enable-hog caused linker
>> errors due to undefined references to HID-related functions:
>>
>> ```
>>> ./configure --disable-hid --enable-hog
>>> make
>> /usr/bin/ld: profiles/input/bluetoothd-hog.o: in function `hog_accept':
>> /home/../bluez/profiles/input/hog.c:184:(.text.hog_accept+0xbb): undefined reference to `input_get_auto_sec'
>> /usr/bin/ld: profiles/input/bluetoothd-hog.o: in function `hog_disconnect':
>> /home/../bluez/profiles/input/hog.c:205:(.text.hog_disconnect+0x12): undefined reference to `input_get_userspace_hid'
>> collect2: error: ld returned 1 exit status
>> make[1]: *** [Makefile:6344: src/bluetoothd] Error 1
>> make: *** [Makefile:4695: all] Error 2
>> ```
>>
>> This patch introduces the HAVE_HID symbol to conditionally call the
>> HID-related code in the HoG plugin only when HID is enabled.
>>
>> Additionally, hog_disconnect() reverts to its pre-[1] behavior when
>> the HID plugin is unavailable.
>>
>> [1] 1782bfd79 input: Add support for UserspaceHID=persist
>>
>> Fixes: https://github.com/bluez/bluez/issues/1228
>> ---
>>   configure.ac         |  3 +++
>>   profiles/input/hog.c | 11 ++++++++++-
>>   2 files changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 1e089aaa7..aa56b7b81 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -194,6 +194,9 @@ AM_CONDITIONAL(NETWORK, test "${enable_network}" != "no")
>>   AC_ARG_ENABLE(hid, AS_HELP_STRING([--disable-hid],
>>                  [disable HID profile]), [enable_hid=${enableval}])
>>   AM_CONDITIONAL(HID, test "${enable_hid}" != "no")
>> +if test "${enable_hid}" != "no"; then
>> +       AC_DEFINE(HAVE_HID, 1, [Define to 1 if you have HID support.])
>> +fi
>>
>>   AC_ARG_ENABLE(hog, AS_HELP_STRING([--disable-hog],
>>                  [disable HoG profile]), [enable_hog=${enableval}])
>> diff --git a/profiles/input/hog.c b/profiles/input/hog.c
>> index 97224f0d1..7ad94c474 100644
>> --- a/profiles/input/hog.c
>> +++ b/profiles/input/hog.c
>> @@ -40,13 +40,16 @@
>>   #include "src/shared/gatt-client.h"
>>   #include "src/plugin.h"
>>
>> -#include "device.h"
>>   #include "suspend.h"
>>   #include "attrib/att.h"
>>   #include "attrib/gattrib.h"
>>   #include "attrib/gatt.h"
>>   #include "hog-lib.h"
>>
>> +#ifdef HAVE_HID
>> +#include "device.h"
>> +#endif /* HAVE_HID */
>> +
>>   struct hog_device {
>>          struct btd_device       *device;
>>          struct bt_hog           *hog;
>> @@ -181,8 +184,10 @@ static int hog_accept(struct btd_service *service)
>>          if (!device_is_bonded(device, btd_device_get_bdaddr_type(device))) {
>>                  struct bt_gatt_client *client;
>>
>> +#ifdef HAVE_HID
>>                  if (!input_get_auto_sec())
>>                          return -ECONNREFUSED;
>> +#endif /* HAVE_HID */
> This doesn't look correct, the input.conf is used to configure both
> HID and HOG, so why do you think it is a good idea to ignore it if HID
> has been disabled? What we probably should do is to move the portion
> of input_init that are HOG specific, e.g. LEAutoSecurity, which is
> what is being used above.
>
>>                  client = btd_device_get_gatt_client(device);
>>                  if (!bt_gatt_client_set_security(client,
>> @@ -202,10 +207,14 @@ static int hog_disconnect(struct btd_service *service)
>>   {
>>          struct hog_device *dev = btd_service_get_user_data(service);
>>
>> +#ifdef HAVE_HID
>>          if (input_get_userspace_hid() == UHID_PERSIST)
>>                  bt_hog_detach(dev->hog, false);
>>          else
>>                  bt_hog_detach(dev->hog, true);
>> +#else
>> +       bt_hog_detach(dev->hog, false);
>> +#endif /* HAVE_HID */
> This part we will probably have to duplicate the parsing of
> UserspaceHID from input_init.
>
>>          btd_service_disconnecting_complete(service, 0);
>>
>> --
>> 2.49.0
>>
>>
>