* RE: Support for config fragments (conf.d style dirs)
2025-12-11 21:13 [PATCH BlueZ 1/1] " Manuel A. Fernandez Montecelo
@ 2025-12-11 21:20 ` bluez.test.bot
0 siblings, 0 replies; 7+ messages in thread
From: bluez.test.bot @ 2025-12-11 21:20 UTC (permalink / raw)
To: linux-bluetooth, mafm
[-- Attachment #1: Type: text/plain, Size: 9275 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=1032437
---Test result---
Test Summary:
CheckPatch PENDING 0.25 seconds
GitLint PENDING 0.34 seconds
BuildEll PASS 20.38 seconds
BluezMake FAIL 11.23 seconds
MakeCheck FAIL 0.27 seconds
MakeDistcheck FAIL 44.00 seconds
CheckValgrind FAIL 10.29 seconds
CheckSmatch FAIL 12.27 seconds
bluezmakeextell FAIL 10.79 seconds
IncrementalBuild PENDING 0.32 seconds
ScanBuild FAIL 13.75 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:
In file included from /usr/include/glib-2.0/glib/gtypes.h:34,
from /usr/include/glib-2.0/glib/galloca.h:32,
from /usr/include/glib-2.0/glib.h:30,
from ./src/eir.h:12,
from src/shared/ad.c:24:
/usr/include/glib-2.0/glib/gversionmacros.h:311:2: error: #error "GLIB_VERSION_MIN_REQUIRED must be <= GLIB_VERSION_CUR_STABLE"
311 | #error "GLIB_VERSION_MIN_REQUIRED must be <= GLIB_VERSION_CUR_STABLE"
| ^~~~~
/usr/include/glib-2.0/glib/gversionmacros.h:314:2: error: #error "GLIB_VERSION_MAX_ALLOWED must be >= GLIB_VERSION_MIN_REQUIRED"
314 | #error "GLIB_VERSION_MAX_ALLOWED must be >= GLIB_VERSION_MIN_REQUIRED"
| ^~~~~
make[1]: *** [Makefile:7770: src/shared/libshared_mainloop_la-ad.lo] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:4201: all] Error 2
##############################
Test: MakeCheck - FAIL
Desc: Run Bluez Make Check
Output:
In file included from /usr/include/glib-2.0/glib/gtypes.h:34,
from /usr/include/glib-2.0/glib/galloca.h:32,
from /usr/include/glib-2.0/glib.h:30,
from client/main.c:23:
/usr/include/glib-2.0/glib/gversionmacros.h:311:2: error: #error "GLIB_VERSION_MIN_REQUIRED must be <= GLIB_VERSION_CUR_STABLE"
311 | #error "GLIB_VERSION_MIN_REQUIRED must be <= GLIB_VERSION_CUR_STABLE"
| ^~~~~
/usr/include/glib-2.0/glib/gversionmacros.h:314:2: error: #error "GLIB_VERSION_MAX_ALLOWED must be >= GLIB_VERSION_MIN_REQUIRED"
314 | #error "GLIB_VERSION_MAX_ALLOWED must be >= GLIB_VERSION_MIN_REQUIRED"
| ^~~~~
make[1]: *** [Makefile:7144: client/main.o] Error 1
make: *** [Makefile:10898: check] Error 2
##############################
Test: MakeDistcheck - FAIL
Desc: Run Bluez Make Distcheck
Output:
../../src/conf_d.c: In function ‘confd_get_valid_files_sorted’:
../../src/conf_d.c:29:37: error: ‘G_REGEX_DEFAULT’ undeclared (first use in this function); did you mean ‘G_REGEX_DOTALL’?
29 | regex = g_regex_new(regex_pattern, G_REGEX_DEFAULT, G_REGEX_MATCH_DEFAULT, &error);
| ^~~~~~~~~~~~~~~
| G_REGEX_DOTALL
../../src/conf_d.c:29:37: note: each undeclared identifier is reported only once for each function it appears in
../../src/conf_d.c:29:54: error: ‘G_REGEX_MATCH_DEFAULT’ undeclared (first use in this function); did you mean ‘G_REGEX_MATCH_NOTEOL’?
29 | regex = g_regex_new(regex_pattern, G_REGEX_DEFAULT, G_REGEX_MATCH_DEFAULT, &error);
| ^~~~~~~~~~~~~~~~~~~~~
| G_REGEX_MATCH_NOTEOL
../../src/conf_d.c:61:3: warning: implicit declaration of function ‘g_ptr_array_sort_values’; did you mean ‘g_ptr_array_sort’? [-Wimplicit-function-declaration]
61 | g_ptr_array_sort_values(ret_confd_files, confd_compare_filenames);
| ^~~~~~~~~~~~~~~~~~~~~~~
| g_ptr_array_sort
make[2]: *** [Makefile:9373: src/bluetoothd-conf_d.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [Makefile:4201: all] Error 2
make: *** [Makefile:10819: distcheck] Error 1
##############################
Test: CheckValgrind - FAIL
Desc: Run Bluez Make Check with Valgrind
Output:
In file included from /usr/include/glib-2.0/glib/gtypes.h:34,
from /usr/include/glib-2.0/glib/galloca.h:32,
from /usr/include/glib-2.0/glib.h:30,
from ./src/eir.h:12,
from src/shared/ad.c:24:
/usr/include/glib-2.0/glib/gversionmacros.h:311:2: error: #error "GLIB_VERSION_MIN_REQUIRED must be <= GLIB_VERSION_CUR_STABLE"
311 | #error "GLIB_VERSION_MIN_REQUIRED must be <= GLIB_VERSION_CUR_STABLE"
| ^~~~~
/usr/include/glib-2.0/glib/gversionmacros.h:314:2: error: #error "GLIB_VERSION_MAX_ALLOWED must be >= GLIB_VERSION_MIN_REQUIRED"
314 | #error "GLIB_VERSION_MAX_ALLOWED must be >= GLIB_VERSION_MIN_REQUIRED"
| ^~~~~
make[1]: *** [Makefile:7770: src/shared/libshared_mainloop_la-ad.lo] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:10898: 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/ad.c: note: in included file (through /usr/include/glib-2.0/glib/gtypes.h, /usr/include/glib-2.0/glib/galloca.h, /usr/include/glib-2.0/glib.h, src/eir.h):
/usr/include/glib-2.0/glib/gversionmacros.h:311:2: error: "GLIB_VERSION_MIN_REQUIRED must be <= GLIB_VERSION_CUR_STABLE"
/usr/include/glib-2.0/glib/gversionmacros.h:314:2: error: "GLIB_VERSION_MAX_ALLOWED must be >= GLIB_VERSION_MIN_REQUIRED"
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.
In file included from /usr/include/glib-2.0/glib/gtypes.h:34,
from /usr/include/glib-2.0/glib/galloca.h:32,
from /usr/include/glib-2.0/glib.h:30,
from ./src/eir.h:12,
from src/shared/ad.c:24:
/usr/include/glib-2.0/glib/gversionmacros.h:311:2: error: #error "GLIB_VERSION_MIN_REQUIRED must be <= GLIB_VERSION_CUR_STABLE"
311 | #error "GLIB_VERSION_MIN_REQUIRED must be <= GLIB_VERSION_CUR_STABLE"
| ^~~~~
/usr/include/glib-2.0/glib/gversionmacros.h:314:2: error: #error "GLIB_VERSION_MAX_ALLOWED must be >= GLIB_VERSION_MIN_REQUIRED"
314 | #error "GLIB_VERSION_MAX_ALLOWED must be >= GLIB_VERSION_MIN_REQUIRED"
| ^~~~~
make[1]: *** [Makefile:7770: src/shared/libshared_mainloop_la-ad.lo] Error 1
make[1]: *** Waiting for unfinished jobs....
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.
make: *** [Makefile:4201: all] Error 2
##############################
Test: bluezmakeextell - FAIL
Desc: Build Bluez with External ELL
Output:
In file included from /usr/include/glib-2.0/glib/gtypes.h:34,
from /usr/include/glib-2.0/glib/galloca.h:32,
from /usr/include/glib-2.0/glib.h:30,
from ./src/eir.h:12,
from src/shared/ad.c:24:
/usr/include/glib-2.0/glib/gversionmacros.h:311:2: error: #error "GLIB_VERSION_MIN_REQUIRED must be <= GLIB_VERSION_CUR_STABLE"
311 | #error "GLIB_VERSION_MIN_REQUIRED must be <= GLIB_VERSION_CUR_STABLE"
| ^~~~~
/usr/include/glib-2.0/glib/gversionmacros.h:314:2: error: #error "GLIB_VERSION_MAX_ALLOWED must be >= GLIB_VERSION_MIN_REQUIRED"
314 | #error "GLIB_VERSION_MAX_ALLOWED must be >= GLIB_VERSION_MIN_REQUIRED"
| ^~~~~
make[1]: *** [Makefile:7770: src/shared/libshared_mainloop_la-ad.lo] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:4201: all] Error 2
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:
##############################
Test: ScanBuild - FAIL
Desc: Run Scan Build
Output:
In file included from /usr/include/glib-2.0/glib/gtypes.h:34,
from /usr/include/glib-2.0/glib/galloca.h:32,
from /usr/include/glib-2.0/glib.h:30,
from ./src/eir.h:12,
from src/shared/ad.c:24:
/usr/include/glib-2.0/glib/gversionmacros.h:311:2: error: #error "GLIB_VERSION_MIN_REQUIRED must be <= GLIB_VERSION_CUR_STABLE"
311 | #error "GLIB_VERSION_MIN_REQUIRED must be <= GLIB_VERSION_CUR_STABLE"
| ^~~~~
/usr/include/glib-2.0/glib/gversionmacros.h:314:2: error: #error "GLIB_VERSION_MAX_ALLOWED must be >= GLIB_VERSION_MIN_REQUIRED"
314 | #error "GLIB_VERSION_MAX_ALLOWED must be >= GLIB_VERSION_MIN_REQUIRED"
| ^~~~~
make[1]: *** [Makefile:7770: src/shared/libshared_mainloop_la-ad.lo] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:4201: all] Error 2
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH BlueZ v2 0/1] Support for config fragments (conf.d style dirs)
@ 2025-12-12 20:22 Manuel A. Fernandez Montecelo
2025-12-12 20:22 ` [PATCH BlueZ v2 1/1] " Manuel A. Fernandez Montecelo
0 siblings, 1 reply; 7+ messages in thread
From: Manuel A. Fernandez Montecelo @ 2025-12-12 20:22 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Manuel A. Fernandez Montecelo
Support config fragments, to read config from conf.d directories.
This is commonly supported by other tools, as an extension of the main
config file(s). It is useful and convenient in several situations,
for example:
* distributions can set different values from the defaults shipped
upstream, without having to modify the config file
* different packages or config-management tools can change config just
by adding, removing or modifying files in that directory; instead of
editing the main config files
Manuel A. Fernandez Montecelo (1):
Support for config fragments (conf.d style dirs)
Makefile.am | 1 +
profiles/input/hog.c | 3 +
profiles/input/manager.c | 3 +
profiles/network/manager.c | 3 +
src/conf_d.c | 202 +++++++++++++++++++++++++++++++++++++
src/conf_d.h | 76 ++++++++++++++
src/main.c | 3 +
7 files changed, 291 insertions(+)
create mode 100644 src/conf_d.c
create mode 100644 src/conf_d.h
--
2.51.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH BlueZ v2 1/1] Support for config fragments (conf.d style dirs)
2025-12-12 20:22 [PATCH BlueZ v2 0/1] Support for config fragments (conf.d style dirs) Manuel A. Fernandez Montecelo
@ 2025-12-12 20:22 ` Manuel A. Fernandez Montecelo
2025-12-12 21:07 ` Luiz Augusto von Dentz
2025-12-12 21:28 ` bluez.test.bot
0 siblings, 2 replies; 7+ messages in thread
From: Manuel A. Fernandez Montecelo @ 2025-12-12 20:22 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Manuel A. Fernandez Montecelo
Support config fragments, to read config from conf.d directories.
Those dirs will be main.conf.d for main.conf, analog for input.conf
and network.conf.
This is commonly supported by other tools, as an extension of the main
config file(s). It is useful and convenient in several situations,
for example:
* distributions can set different values from the defaults shipped
upstream, without having to modify the config file
* different packages or config-management tools can change config just
by adding, removing or modifying files in that directory; instead of
editing the main config files
The main or base config files will be processed first, and then files
in the conf.d dirs, if existing.
When reading these config files in conf.d dirs, they override values
for keys in the base config files (or default config set in code).
For example, for "main.conf" the directory to be processed will be
"main.conf.d", in the same basedir as the config file
(e.g. /etc/main.conf, /etc/main.conf.d/). The same for input.conf and
network.conf.
Within the conf.d directory, the format of the filename should be
'^([0-9][0-9])-([a-zA-Z0-9-_])*\.conf$', that is, starting with "00-"
to "99-", ending in ".conf", and with a mix of alphanumeric characters
with dashes and underscores in between. For example:
'01-override-general-secureconnections.conf'.
Files named differently will not be considered, and accepting groups
or keys not present in the base config depends on the code, currently
set to "NOT accept new groups" but "YES to accept new keys". This is
because the base config files contain all the groups, but most keys
are commented-out, with the values set in code.
The candidate files within the given directory are sorted (with
g_strcmp0(), so the ordering will be as with strcmp()). The
configuration in the files being processed later will override
previous config, in particular the main/base config files, but also
the one from previous files processed, if the Group and Key coincide.
For example, consider 'main.conf' that contains the defaults:
[General]
DiscoverableTimeout=0
PairableTimeout=0
and there is a file 'main.conf.d/70-default-timeouts-vendor.conf'
containing settings for these keys:
[General]
DiscoverableTimeout=30
PairableTimeout=30
and another 'main.conf.d/99-default-timeouts-local.conf'
containing settings only for 'PairableTimeout':
[General]
PairableTimeout=15
What happens is:
1) First, the 'main.conf' is processed as usual;
2) then 'main.conf.d/70-default-timeouts-vendor.conf' is processed,
overriding the two values from the main config file with the given
values;
3) and finally 'main.conf.d/99-default-timeouts-local.conf' is
processed, overriding once again only 'PairableTimeout'.
The final, effective values are:
DiscoverableTimeout=30
PairableTimeout=15
---
Makefile.am | 1 +
profiles/input/hog.c | 3 +
profiles/input/manager.c | 3 +
profiles/network/manager.c | 3 +
src/conf_d.c | 202 +++++++++++++++++++++++++++++++++++++
src/conf_d.h | 76 ++++++++++++++
src/main.c | 3 +
7 files changed, 291 insertions(+)
create mode 100644 src/conf_d.c
create mode 100644 src/conf_d.h
diff --git a/Makefile.am b/Makefile.am
index e152ae648..f8516bacd 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -307,6 +307,7 @@ pkglibexec_PROGRAMS += src/bluetoothd
bluetoothd_internal_sources = \
$(attrib_sources) $(btio_sources) \
src/log.h src/log.c \
+ src/conf_d.h src/conf_d.c \
src/backtrace.h src/backtrace.c \
src/rfkill.c src/btd.h src/sdpd.h \
src/sdpd-server.c src/sdpd-request.c \
diff --git a/profiles/input/hog.c b/profiles/input/hog.c
index f50a0f217..6d2348c26 100644
--- a/profiles/input/hog.c
+++ b/profiles/input/hog.c
@@ -39,6 +39,7 @@
#include "src/shared/att.h"
#include "src/shared/gatt-client.h"
#include "src/plugin.h"
+#include "src/conf_d.h"
#include "suspend.h"
#include "attrib/att.h"
@@ -246,6 +247,8 @@ static void hog_read_config(void)
return;
}
+ confd_process_config(config, filename, FALSE, TRUE);
+
config_auto_sec = g_key_file_get_boolean(config, "General",
"LEAutoSecurity", &err);
if (!err) {
diff --git a/profiles/input/manager.c b/profiles/input/manager.c
index 0fcd6728c..68691ec52 100644
--- a/profiles/input/manager.c
+++ b/profiles/input/manager.c
@@ -27,6 +27,7 @@
#include "src/device.h"
#include "src/profile.h"
#include "src/service.h"
+#include "src/conf_d.h"
#include "device.h"
#include "server.h"
@@ -75,6 +76,8 @@ static GKeyFile *load_config_file(const char *file)
return NULL;
}
+ confd_process_config(keyfile, file, FALSE, TRUE);
+
return keyfile;
}
diff --git a/profiles/network/manager.c b/profiles/network/manager.c
index 693547d45..e9b620b0f 100644
--- a/profiles/network/manager.c
+++ b/profiles/network/manager.c
@@ -28,6 +28,7 @@
#include "src/device.h"
#include "src/profile.h"
#include "src/service.h"
+#include "src/conf_d.h"
#include "bnep.h"
#include "connection.h"
@@ -47,6 +48,8 @@ static void read_config(const char *file)
goto done;
}
+ confd_process_config(keyfile, file, FALSE, TRUE);
+
conf_security = !g_key_file_get_boolean(keyfile, "General",
"DisableSecurity", &err);
if (err) {
diff --git a/src/conf_d.c b/src/conf_d.c
new file mode 100644
index 000000000..878fb9157
--- /dev/null
+++ b/src/conf_d.c
@@ -0,0 +1,202 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2025 Valve Corporation
+ *
+ */
+
+#include "conf_d.h"
+
+#include "src/log.h"
+
+static gint confd_compare_filenames(gconstpointer a, gconstpointer b)
+{
+ return g_strcmp0(*(const gchar **)(a), *(const gchar **)(b));
+}
+
+static GPtrArray *confd_get_valid_files_sorted(const gchar *confd_path)
+{
+ const char *regex_pattern = "^([0-9][0-9])-([a-zA-Z0-9-_])*\\.conf$";
+ g_autoptr(GRegex) regex = NULL;
+ g_autoptr(GPtrArray) ret_confd_files = NULL;
+ GDir *dir = NULL;
+ GError *error = NULL;
+ const gchar *filename = NULL;
+
+ regex = g_regex_new(regex_pattern, 0, 0, &error);
+ if (!regex) {
+ DBG("Invalid regex: %s", error->message);
+ g_clear_error(&error);
+ return NULL;
+ }
+
+ dir = g_dir_open(confd_path, 0, &error);
+ if (!dir) {
+ DBG("%s", error->message);
+ g_clear_error(&error);
+ return NULL;
+ }
+
+ ret_confd_files = g_ptr_array_new_full(0, g_free);
+
+ while ((filename = g_dir_read_name(dir)) != NULL) {
+ g_autofree gchar *file_path = NULL;
+
+ if (!g_regex_match(regex, filename, 0, NULL)) {
+ DBG("Ignoring file in conf.d dir: '%s'", filename);
+ continue;
+ }
+
+ file_path = g_build_filename(confd_path, filename, NULL);
+ if (file_path)
+ g_ptr_array_add(ret_confd_files, g_strdup(file_path));
+ }
+
+ g_dir_close(dir);
+
+ if (ret_confd_files && ret_confd_files->len > 0) {
+ g_ptr_array_sort(ret_confd_files, confd_compare_filenames);
+
+ DBG("Will consider additional config files (in order):");
+ for (guint i = 0; i < ret_confd_files->len; i++) {
+ DBG(" - %s",
+ (const gchar *)(g_ptr_array_index(ret_confd_files,
+ i)));
+ }
+
+ return g_ptr_array_ref(ret_confd_files);
+ } else {
+ g_ptr_array_free(ret_confd_files, TRUE);
+ ret_confd_files = NULL;
+ return NULL;
+ }
+}
+
+static void confd_override_config(GKeyFile *keyfile,
+ const gchar *new_conf_file_path,
+ gboolean accept_new_groups,
+ gboolean accept_new_keys)
+{
+ g_autoptr(GKeyFile) new_keyfile = NULL;
+ gchar **existing_groups = NULL;
+ gchar **groups = NULL;
+ gchar **keys = NULL;
+ gsize existing_groups_size = 0;
+ gsize groups_size = 0;
+ gsize keys_size = 0;
+ g_autoptr(GError) error = NULL;
+
+ new_keyfile = g_key_file_new();
+ if (!g_key_file_load_from_file(new_keyfile, new_conf_file_path,
+ G_KEY_FILE_NONE, &error)) {
+ if (error) {
+ warn("%s", error->message);
+ g_clear_error(&error);
+ }
+ return;
+ }
+
+ existing_groups = g_key_file_get_groups(keyfile, &existing_groups_size);
+
+ groups = g_key_file_get_groups(new_keyfile, &groups_size);
+ for (gsize gi = 0; gi < groups_size; gi++) {
+ bool match = false;
+ const gchar *group = groups[gi];
+
+ for (gsize egi = 0; egi < existing_groups_size; egi++) {
+ if (g_str_equal(group, existing_groups[egi])) {
+ match = true;
+ break;
+ }
+ }
+
+ if (!match) {
+ if (accept_new_groups == FALSE) {
+ warn("Skipping group '%s' in '%s' "
+ "not known in previous config",
+ group, new_conf_file_path);
+ continue;
+ } else {
+ DBG("Accepting group '%s' in '%s' "
+ "not known in previous config",
+ group, new_conf_file_path);
+ }
+ }
+
+ keys = g_key_file_get_keys(new_keyfile, group, &keys_size,
+ NULL);
+ if (keys == NULL) {
+ DBG("No keys found in '%s' for group '%s'",
+ new_conf_file_path, group);
+ continue;
+ }
+
+ for (gsize ki = 0; ki < keys_size; ki++) {
+ const gchar *key = keys[ki];
+ g_autofree gchar *value = NULL;
+ g_autofree gchar *old_value = NULL;
+
+ value = g_key_file_get_value(new_keyfile, group, key,
+ NULL);
+ if (!value)
+ continue;
+
+ old_value =
+ g_key_file_get_value(keyfile, group, key, NULL);
+ if (old_value != NULL) {
+ DBG("Overriding config value from "
+ "conf.d file: [%s] %s: '%s'->'%s'",
+ group, key, old_value, value);
+ g_key_file_set_value(keyfile, group, key,
+ value);
+ } else {
+ if (accept_new_keys == TRUE) {
+ DBG("Adding new config value from "
+ "conf.d file: [%s] %s: '%s'",
+ group, key, value);
+ g_key_file_set_value(keyfile, group,
+ key, value);
+ } else {
+ DBG("Ignoring config value from "
+ "conf.d, unknown keys not allowed: "
+ "[%s] %s: '%s'",
+ group, key, value);
+ }
+ }
+ }
+ g_strfreev(keys);
+ }
+ g_strfreev(groups);
+ g_strfreev(existing_groups);
+}
+
+void confd_process_config(GKeyFile *keyfile, const gchar *base_conf_file_path,
+ gboolean accept_new_groups, gboolean accept_new_keys)
+{
+ g_autofree gchar *confd_path = NULL;
+ g_autoptr(GPtrArray) confd_files = NULL;
+
+ confd_path = g_strconcat(base_conf_file_path, ".d", NULL);
+
+ if (!g_file_test(confd_path,
+ (G_FILE_TEST_EXISTS | G_FILE_TEST_IS_DIR))) {
+ DBG("'%s' does not exist or not a directory", confd_path);
+ return;
+ }
+
+ confd_files = confd_get_valid_files_sorted(confd_path);
+
+ if (confd_files && confd_files->len > 0) {
+ for (guint i = 0; i < confd_files->len; i++) {
+ const gchar *confd_file =
+ (const gchar *)(g_ptr_array_index(confd_files,
+ i));
+ DBG("Processing config file: '%s'", confd_file);
+ confd_override_config(keyfile, confd_file,
+ accept_new_groups,
+ accept_new_keys);
+ }
+ }
+}
diff --git a/src/conf_d.h b/src/conf_d.h
new file mode 100644
index 000000000..812966740
--- /dev/null
+++ b/src/conf_d.h
@@ -0,0 +1,76 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2025 Valve Corporation
+ *
+ */
+
+#include <glib.h>
+
+/**
+ * confd_process_config:
+ *
+ * @keyfile: keyfile already initialized and parsed
+ *
+ * @base_conf_file_path: base config file (e.g. /etc/bluetooth/main.conf,
+ * input.conf, network.conf). The directory to be processed will be same path
+ * with ".d" appended.
+ *
+ * @accept_new_groups: whether to accept groups not appearing in the base config
+ * file
+ *
+ * @accept_new_keys: whether to accept keys not appearing in the base config
+ * file
+ *
+ * Helper function to process config files in conf.d style dirs (config
+ * fragments), overriding values for keys in the base config files (or default
+ * config set in code). For example, for "main.conf" the directory to be
+ * processed will be "main.conf.d", in the same basedir as the config file.
+ *
+ * Within the .d directory, the format of the filename should be
+ * '^([0-9][0-9])-([a-zA-Z0-9-_])*\.conf$', that is, starting with "00-" to
+ * "99-", ending in ".conf", and with a mix of alphanumeric characters with
+ * dashes and underscores in between. For example:
+ * '01-override-general-secureconnections.conf'.
+ *
+ * Files named differently will not be considered, and accepting groups or keys
+ * not present in the base config depends on the function arguments.
+ *
+ * The candidate files within the given directory are sorted (with g_strcmp0(),
+ * so the ordering will be as with strcmp()). The configuration in the files
+ * being processed later will override previous config, in particular the main
+ * config, but also the one from previous files processed, if the Group and Key
+ * coincide.
+ *
+ * For example, consider 'main.conf' that contains the defaults:
+ * [General]
+ * DiscoverableTimeout=0
+ * PairableTimeout=0
+ *
+ * and there is a file 'main.conf.d/70-default-timeouts-vendor.conf'
+ * containing settings for these keys:
+ * [General]
+ * DiscoverableTimeout=30
+ * PairableTimeout=30
+ *
+ * and another 'main.conf.d/99-default-timeouts-local.conf'
+ * containing settings only for 'PairableTimeout':
+ * [General]
+ * PairableTimeout=15
+ *
+ * What happens is:
+ * 1) First, the 'main.conf' is processed as usual;
+ * 2) then 'main.conf.d/70-default-timeouts-vendor.conf' is processed,
+ * overriding the two values from the main config file with the given values;
+ * 3) and finally 'main.conf.d/99-default-timeouts-local.conf' is processed,
+ * overriding once again only 'PairableTimeout'.
+ *
+ * The final, effective values are:
+ *
+ * DiscoverableTimeout=30
+ *
+ **/
+void confd_process_config(GKeyFile *keyfile, const gchar *base_conf_file_path,
+ gboolean accept_new_groups, gboolean accept_new_keys);
diff --git a/src/main.c b/src/main.c
index 61e5ef983..79866616e 100644
--- a/src/main.c
+++ b/src/main.c
@@ -54,6 +54,7 @@
#include "dbus-common.h"
#include "agent.h"
#include "profile.h"
+#include "conf_d.h"
#define BLUEZ_NAME "org.bluez"
@@ -284,6 +285,8 @@ static GKeyFile *load_config(const char *name)
return NULL;
}
+ confd_process_config(keyfile, main_conf_file_path, FALSE, TRUE);
+
return keyfile;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH BlueZ v2 1/1] Support for config fragments (conf.d style dirs)
2025-12-12 20:22 ` [PATCH BlueZ v2 1/1] " Manuel A. Fernandez Montecelo
@ 2025-12-12 21:07 ` Luiz Augusto von Dentz
2025-12-12 23:41 ` Manuel A. Fernandez Montecelo
2025-12-12 21:28 ` bluez.test.bot
1 sibling, 1 reply; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2025-12-12 21:07 UTC (permalink / raw)
To: Manuel A. Fernandez Montecelo; +Cc: linux-bluetooth
Hi Manuel,
On Fri, Dec 12, 2025 at 3:23 PM Manuel A. Fernandez Montecelo
<mafm@igalia.com> wrote:
>
> Support config fragments, to read config from conf.d directories.
> Those dirs will be main.conf.d for main.conf, analog for input.conf
> and network.conf.
>
> This is commonly supported by other tools, as an extension of the main
> config file(s). It is useful and convenient in several situations,
> for example:
>
> * distributions can set different values from the defaults shipped
> upstream, without having to modify the config file
>
> * different packages or config-management tools can change config just
> by adding, removing or modifying files in that directory; instead of
> editing the main config files
>
> The main or base config files will be processed first, and then files
> in the conf.d dirs, if existing.
>
> When reading these config files in conf.d dirs, they override values
> for keys in the base config files (or default config set in code).
> For example, for "main.conf" the directory to be processed will be
> "main.conf.d", in the same basedir as the config file
> (e.g. /etc/main.conf, /etc/main.conf.d/). The same for input.conf and
> network.conf.
>
> Within the conf.d directory, the format of the filename should be
> '^([0-9][0-9])-([a-zA-Z0-9-_])*\.conf$', that is, starting with "00-"
> to "99-", ending in ".conf", and with a mix of alphanumeric characters
> with dashes and underscores in between. For example:
> '01-override-general-secureconnections.conf'.
>
> Files named differently will not be considered, and accepting groups
> or keys not present in the base config depends on the code, currently
> set to "NOT accept new groups" but "YES to accept new keys". This is
> because the base config files contain all the groups, but most keys
> are commented-out, with the values set in code.
>
> The candidate files within the given directory are sorted (with
> g_strcmp0(), so the ordering will be as with strcmp()). The
> configuration in the files being processed later will override
> previous config, in particular the main/base config files, but also
> the one from previous files processed, if the Group and Key coincide.
>
> For example, consider 'main.conf' that contains the defaults:
>
> [General]
> DiscoverableTimeout=0
> PairableTimeout=0
>
> and there is a file 'main.conf.d/70-default-timeouts-vendor.conf'
> containing settings for these keys:
>
> [General]
> DiscoverableTimeout=30
> PairableTimeout=30
>
> and another 'main.conf.d/99-default-timeouts-local.conf'
> containing settings only for 'PairableTimeout':
>
> [General]
> PairableTimeout=15
>
> What happens is:
> 1) First, the 'main.conf' is processed as usual;
> 2) then 'main.conf.d/70-default-timeouts-vendor.conf' is processed,
> overriding the two values from the main config file with the given
> values;
> 3) and finally 'main.conf.d/99-default-timeouts-local.conf' is
> processed, overriding once again only 'PairableTimeout'.
>
> The final, effective values are:
>
> DiscoverableTimeout=30
> PairableTimeout=15
> ---
> Makefile.am | 1 +
> profiles/input/hog.c | 3 +
> profiles/input/manager.c | 3 +
> profiles/network/manager.c | 3 +
> src/conf_d.c | 202 +++++++++++++++++++++++++++++++++++++
> src/conf_d.h | 76 ++++++++++++++
> src/main.c | 3 +
> 7 files changed, 291 insertions(+)
> create mode 100644 src/conf_d.c
> create mode 100644 src/conf_d.h
>
> diff --git a/Makefile.am b/Makefile.am
> index e152ae648..f8516bacd 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -307,6 +307,7 @@ pkglibexec_PROGRAMS += src/bluetoothd
> bluetoothd_internal_sources = \
> $(attrib_sources) $(btio_sources) \
> src/log.h src/log.c \
> + src/conf_d.h src/conf_d.c \
> src/backtrace.h src/backtrace.c \
> src/rfkill.c src/btd.h src/sdpd.h \
> src/sdpd-server.c src/sdpd-request.c \
> diff --git a/profiles/input/hog.c b/profiles/input/hog.c
> index f50a0f217..6d2348c26 100644
> --- a/profiles/input/hog.c
> +++ b/profiles/input/hog.c
> @@ -39,6 +39,7 @@
> #include "src/shared/att.h"
> #include "src/shared/gatt-client.h"
> #include "src/plugin.h"
> +#include "src/conf_d.h"
>
> #include "suspend.h"
> #include "attrib/att.h"
> @@ -246,6 +247,8 @@ static void hog_read_config(void)
> return;
> }
>
> + confd_process_config(config, filename, FALSE, TRUE);
> +
> config_auto_sec = g_key_file_get_boolean(config, "General",
> "LEAutoSecurity", &err);
> if (!err) {
> diff --git a/profiles/input/manager.c b/profiles/input/manager.c
> index 0fcd6728c..68691ec52 100644
> --- a/profiles/input/manager.c
> +++ b/profiles/input/manager.c
> @@ -27,6 +27,7 @@
> #include "src/device.h"
> #include "src/profile.h"
> #include "src/service.h"
> +#include "src/conf_d.h"
>
> #include "device.h"
> #include "server.h"
> @@ -75,6 +76,8 @@ static GKeyFile *load_config_file(const char *file)
> return NULL;
> }
>
> + confd_process_config(keyfile, file, FALSE, TRUE);
> +
> return keyfile;
> }
>
> diff --git a/profiles/network/manager.c b/profiles/network/manager.c
> index 693547d45..e9b620b0f 100644
> --- a/profiles/network/manager.c
> +++ b/profiles/network/manager.c
> @@ -28,6 +28,7 @@
> #include "src/device.h"
> #include "src/profile.h"
> #include "src/service.h"
> +#include "src/conf_d.h"
>
> #include "bnep.h"
> #include "connection.h"
> @@ -47,6 +48,8 @@ static void read_config(const char *file)
> goto done;
> }
>
> + confd_process_config(keyfile, file, FALSE, TRUE);
> +
> conf_security = !g_key_file_get_boolean(keyfile, "General",
> "DisableSecurity", &err);
> if (err) {
> diff --git a/src/conf_d.c b/src/conf_d.c
> new file mode 100644
> index 000000000..878fb9157
> --- /dev/null
> +++ b/src/conf_d.c
> @@ -0,0 +1,202 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + *
> + * BlueZ - Bluetooth protocol stack for Linux
> + *
> + * Copyright (C) 2025 Valve Corporation
> + *
> + */
> +
> +#include "conf_d.h"
> +
> +#include "src/log.h"
> +
> +static gint confd_compare_filenames(gconstpointer a, gconstpointer b)
> +{
> + return g_strcmp0(*(const gchar **)(a), *(const gchar **)(b));
> +}
> +
> +static GPtrArray *confd_get_valid_files_sorted(const gchar *confd_path)
> +{
> + const char *regex_pattern = "^([0-9][0-9])-([a-zA-Z0-9-_])*\\.conf$";
> + g_autoptr(GRegex) regex = NULL;
> + g_autoptr(GPtrArray) ret_confd_files = NULL;
> + GDir *dir = NULL;
> + GError *error = NULL;
> + const gchar *filename = NULL;
> +
> + regex = g_regex_new(regex_pattern, 0, 0, &error);
> + if (!regex) {
> + DBG("Invalid regex: %s", error->message);
> + g_clear_error(&error);
> + return NULL;
> + }
> +
> + dir = g_dir_open(confd_path, 0, &error);
> + if (!dir) {
> + DBG("%s", error->message);
> + g_clear_error(&error);
> + return NULL;
> + }
> +
> + ret_confd_files = g_ptr_array_new_full(0, g_free);
> +
> + while ((filename = g_dir_read_name(dir)) != NULL) {
> + g_autofree gchar *file_path = NULL;
> +
> + if (!g_regex_match(regex, filename, 0, NULL)) {
> + DBG("Ignoring file in conf.d dir: '%s'", filename);
> + continue;
> + }
> +
> + file_path = g_build_filename(confd_path, filename, NULL);
> + if (file_path)
> + g_ptr_array_add(ret_confd_files, g_strdup(file_path));
> + }
> +
> + g_dir_close(dir);
> +
> + if (ret_confd_files && ret_confd_files->len > 0) {
> + g_ptr_array_sort(ret_confd_files, confd_compare_filenames);
> +
> + DBG("Will consider additional config files (in order):");
> + for (guint i = 0; i < ret_confd_files->len; i++) {
> + DBG(" - %s",
> + (const gchar *)(g_ptr_array_index(ret_confd_files,
> + i)));
> + }
> +
> + return g_ptr_array_ref(ret_confd_files);
> + } else {
> + g_ptr_array_free(ret_confd_files, TRUE);
> + ret_confd_files = NULL;
> + return NULL;
> + }
> +}
> +
> +static void confd_override_config(GKeyFile *keyfile,
> + const gchar *new_conf_file_path,
> + gboolean accept_new_groups,
> + gboolean accept_new_keys)
> +{
> + g_autoptr(GKeyFile) new_keyfile = NULL;
> + gchar **existing_groups = NULL;
> + gchar **groups = NULL;
> + gchar **keys = NULL;
> + gsize existing_groups_size = 0;
> + gsize groups_size = 0;
> + gsize keys_size = 0;
> + g_autoptr(GError) error = NULL;
> +
> + new_keyfile = g_key_file_new();
> + if (!g_key_file_load_from_file(new_keyfile, new_conf_file_path,
> + G_KEY_FILE_NONE, &error)) {
> + if (error) {
> + warn("%s", error->message);
> + g_clear_error(&error);
> + }
> + return;
> + }
> +
> + existing_groups = g_key_file_get_groups(keyfile, &existing_groups_size);
> +
> + groups = g_key_file_get_groups(new_keyfile, &groups_size);
> + for (gsize gi = 0; gi < groups_size; gi++) {
> + bool match = false;
> + const gchar *group = groups[gi];
> +
> + for (gsize egi = 0; egi < existing_groups_size; egi++) {
> + if (g_str_equal(group, existing_groups[egi])) {
> + match = true;
> + break;
> + }
> + }
> +
> + if (!match) {
> + if (accept_new_groups == FALSE) {
> + warn("Skipping group '%s' in '%s' "
> + "not known in previous config",
> + group, new_conf_file_path);
> + continue;
> + } else {
> + DBG("Accepting group '%s' in '%s' "
> + "not known in previous config",
> + group, new_conf_file_path);
> + }
> + }
> +
> + keys = g_key_file_get_keys(new_keyfile, group, &keys_size,
> + NULL);
> + if (keys == NULL) {
> + DBG("No keys found in '%s' for group '%s'",
> + new_conf_file_path, group);
> + continue;
> + }
> +
> + for (gsize ki = 0; ki < keys_size; ki++) {
> + const gchar *key = keys[ki];
> + g_autofree gchar *value = NULL;
> + g_autofree gchar *old_value = NULL;
> +
> + value = g_key_file_get_value(new_keyfile, group, key,
> + NULL);
> + if (!value)
> + continue;
> +
> + old_value =
> + g_key_file_get_value(keyfile, group, key, NULL);
> + if (old_value != NULL) {
> + DBG("Overriding config value from "
> + "conf.d file: [%s] %s: '%s'->'%s'",
> + group, key, old_value, value);
> + g_key_file_set_value(keyfile, group, key,
> + value);
> + } else {
> + if (accept_new_keys == TRUE) {
> + DBG("Adding new config value from "
> + "conf.d file: [%s] %s: '%s'",
> + group, key, value);
> + g_key_file_set_value(keyfile, group,
> + key, value);
> + } else {
> + DBG("Ignoring config value from "
> + "conf.d, unknown keys not allowed: "
> + "[%s] %s: '%s'",
> + group, key, value);
> + }
> + }
> + }
> + g_strfreev(keys);
> + }
> + g_strfreev(groups);
> + g_strfreev(existing_groups);
> +}
> +
> +void confd_process_config(GKeyFile *keyfile, const gchar *base_conf_file_path,
> + gboolean accept_new_groups, gboolean accept_new_keys)
> +{
> + g_autofree gchar *confd_path = NULL;
> + g_autoptr(GPtrArray) confd_files = NULL;
> +
> + confd_path = g_strconcat(base_conf_file_path, ".d", NULL);
> +
> + if (!g_file_test(confd_path,
> + (G_FILE_TEST_EXISTS | G_FILE_TEST_IS_DIR))) {
> + DBG("'%s' does not exist or not a directory", confd_path);
> + return;
> + }
> +
> + confd_files = confd_get_valid_files_sorted(confd_path);
> +
> + if (confd_files && confd_files->len > 0) {
> + for (guint i = 0; i < confd_files->len; i++) {
> + const gchar *confd_file =
> + (const gchar *)(g_ptr_array_index(confd_files,
> + i));
> + DBG("Processing config file: '%s'", confd_file);
> + confd_override_config(keyfile, confd_file,
> + accept_new_groups,
> + accept_new_keys);
> + }
> + }
> +}
> diff --git a/src/conf_d.h b/src/conf_d.h
> new file mode 100644
> index 000000000..812966740
> --- /dev/null
> +++ b/src/conf_d.h
> @@ -0,0 +1,76 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + *
> + * BlueZ - Bluetooth protocol stack for Linux
> + *
> + * Copyright (C) 2025 Valve Corporation
> + *
> + */
> +
> +#include <glib.h>
> +
> +/**
> + * confd_process_config:
> + *
> + * @keyfile: keyfile already initialized and parsed
> + *
> + * @base_conf_file_path: base config file (e.g. /etc/bluetooth/main.conf,
> + * input.conf, network.conf). The directory to be processed will be same path
> + * with ".d" appended.
> + *
> + * @accept_new_groups: whether to accept groups not appearing in the base config
> + * file
> + *
> + * @accept_new_keys: whether to accept keys not appearing in the base config
> + * file
> + *
> + * Helper function to process config files in conf.d style dirs (config
> + * fragments), overriding values for keys in the base config files (or default
> + * config set in code). For example, for "main.conf" the directory to be
> + * processed will be "main.conf.d", in the same basedir as the config file.
> + *
> + * Within the .d directory, the format of the filename should be
> + * '^([0-9][0-9])-([a-zA-Z0-9-_])*\.conf$', that is, starting with "00-" to
> + * "99-", ending in ".conf", and with a mix of alphanumeric characters with
> + * dashes and underscores in between. For example:
> + * '01-override-general-secureconnections.conf'.
> + *
> + * Files named differently will not be considered, and accepting groups or keys
> + * not present in the base config depends on the function arguments.
> + *
> + * The candidate files within the given directory are sorted (with g_strcmp0(),
> + * so the ordering will be as with strcmp()). The configuration in the files
> + * being processed later will override previous config, in particular the main
> + * config, but also the one from previous files processed, if the Group and Key
> + * coincide.
> + *
> + * For example, consider 'main.conf' that contains the defaults:
> + * [General]
> + * DiscoverableTimeout=0
> + * PairableTimeout=0
> + *
> + * and there is a file 'main.conf.d/70-default-timeouts-vendor.conf'
> + * containing settings for these keys:
> + * [General]
> + * DiscoverableTimeout=30
> + * PairableTimeout=30
> + *
> + * and another 'main.conf.d/99-default-timeouts-local.conf'
> + * containing settings only for 'PairableTimeout':
> + * [General]
> + * PairableTimeout=15
> + *
> + * What happens is:
> + * 1) First, the 'main.conf' is processed as usual;
> + * 2) then 'main.conf.d/70-default-timeouts-vendor.conf' is processed,
> + * overriding the two values from the main config file with the given values;
> + * 3) and finally 'main.conf.d/99-default-timeouts-local.conf' is processed,
> + * overriding once again only 'PairableTimeout'.
> + *
> + * The final, effective values are:
> + *
> + * DiscoverableTimeout=30
And PairableTimeout=15, right?
> + *
> + **/
> +void confd_process_config(GKeyFile *keyfile, const gchar *base_conf_file_path,
> + gboolean accept_new_groups, gboolean accept_new_keys);
> diff --git a/src/main.c b/src/main.c
> index 61e5ef983..79866616e 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -54,6 +54,7 @@
> #include "dbus-common.h"
> #include "agent.h"
> #include "profile.h"
> +#include "conf_d.h"
>
> #define BLUEZ_NAME "org.bluez"
>
> @@ -284,6 +285,8 @@ static GKeyFile *load_config(const char *name)
> return NULL;
> }
>
> + confd_process_config(keyfile, main_conf_file_path, FALSE, TRUE);
After a second look this does seem to completely ignore the formatting
of logging when parsing main.conf, also I do wonder why you didn't
just reuse load_config to parse the files in conf.d, we will need to
adjust the use of main_conf_file_path perhaps to just conf_file_path,
or better yet add a second argument for the path represented by the
GKeyFile, but I believe this is a lot more straightforward overwriting
the same GKeyFile with all the values from conf.d, actually I don't
think this really checks the existing groups and keys because it will
initially be based on the existing main.conf which can be wrong
already, but you will only be able to detect that while in
parse_config.
So except if I miss something above, I think we should really parse
each config separately, and not just load and overwrite.
> return keyfile;
> }
>
> --
> 2.51.0
>
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: Support for config fragments (conf.d style dirs)
2025-12-12 20:22 ` [PATCH BlueZ v2 1/1] " Manuel A. Fernandez Montecelo
2025-12-12 21:07 ` Luiz Augusto von Dentz
@ 2025-12-12 21:28 ` bluez.test.bot
1 sibling, 0 replies; 7+ messages in thread
From: bluez.test.bot @ 2025-12-12 21:28 UTC (permalink / raw)
To: linux-bluetooth, mafm
[-- Attachment #1: Type: text/plain, Size: 1262 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=1032731
---Test result---
Test Summary:
CheckPatch PENDING 0.39 seconds
GitLint PENDING 0.38 seconds
BuildEll PASS 20.27 seconds
BluezMake PASS 653.03 seconds
MakeCheck PASS 22.32 seconds
MakeDistcheck PASS 244.33 seconds
CheckValgrind PASS 304.69 seconds
CheckSmatch PASS 351.98 seconds
bluezmakeextell PASS 183.84 seconds
IncrementalBuild PENDING 0.31 seconds
ScanBuild PASS 1045.35 seconds
Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:
##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH BlueZ v2 1/1] Support for config fragments (conf.d style dirs)
2025-12-12 21:07 ` Luiz Augusto von Dentz
@ 2025-12-12 23:41 ` Manuel A. Fernandez Montecelo
0 siblings, 0 replies; 7+ messages in thread
From: Manuel A. Fernandez Montecelo @ 2025-12-12 23:41 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Luiz Augusto von Dentz
Hi,
On 12/12/2025 22:07, Luiz Augusto von Dentz wrote:
>> + * The final, effective values are:
>> + *
>> + * DiscoverableTimeout=30
>
> And PairableTimeout=15, right?
Yes, good catch, thanks. I suppose that it got lost at some point when
I was reformatting the paragraphs.
>> @@ -284,6 +285,8 @@ static GKeyFile *load_config(const char *name)
>> return NULL;
>> }
>>
>> + confd_process_config(keyfile, main_conf_file_path, FALSE, TRUE);
>
> After a second look this does seem to completely ignore the formatting
> of logging when parsing main.conf,
I don't understand this sentence. Do you mean that it doesn't emit log
errors when the extra config files cannot be parsed, like in src/main.c:279?
There's a check at the beginning of confd_override_config(), it emits a
warning (not an error) if it fails to load the file.
Or do you mean the different, finer-grained checks and logging messages
when parsing keys and values?
> also I do wonder why you didn't
> just reuse load_config to parse the files in conf.d, we will need to
> adjust the use of main_conf_file_path perhaps to just conf_file_path,
> or better yet add a second argument for the path represented by the
> GKeyFile, but I believe this is a lot more straightforward overwriting
> the same GKeyFile with all the values from conf.d,
(As means of explaining why I did it that way, not as a reason to change
it -- I'm fine with doing it in other ways)
I was trying different approaches, I think that I started doing what you
say when looking at src/main.c.
But then I thought better to implement it in a self-contained way, so
the changes to the existing code were minimal, and it could be reused in
different places (profile/input/manager.c, etc), without having to
replicate the walking over the extra files.
Also, with the config merged into a single GKeyFile in-place, it will
avoid problems like for example if the parsing_*() functions assume that
there's only one config file, and in each pass (loading each config
file) setting default values if they don't find in the current file
being parsed, even if it was set in a previous pass. All such places
parsing config would have to be mindful that there could be multiple
files, each containing partial config, and so the higher-level code has
to do the job of merging the partial config. (Not sure if it makes sense?)
And perhaps a minor nuisance, special cases like use of
getenv("CONFIGURATION_DIRECTORY") will have to be taken into account in
more places.
So, all in all, I thought that for the existing code and future changes,
it would be simpler if the GKeyFile returned from load_config() (or
equivalent in other sections of code) contained the base+extended
configuration all in one GKeyFile, with the extended config injected in
that same place of the base file, so it would work transparently for the
rest of the code parsing it.
> actually I don't
> think this really checks the existing groups and keys because it will
> initially be based on the existing main.conf which can be wrong
> already, but you will only be able to detect that while in
> parse_config.
Yes, the new code assumes that the base config file is correct, and it
would paper over errors.
For example, if the base file (e.g. main.conf) contains a wrong group
not known to the existing bluetooth code, and the extra config contain
keys in that group, it will load the new key values into GKeyFile. My
understanding (I didn't check thoroughly) is that the rest of the
bluetooh code will ignore these, as it will try to parse only groups and
keys that it knows about -- it's hardcoded.
Also, in the current implementation, it will mask some problems that
could be present in the config files. For example if main.conf contains
"PairableTimeout = garbage" instead of an integer, and
main.conf.d/10-override-pairabletimeout.conf contains "PairableTimeout =
garbageagain", and another
main.conf.d/99-override-pairabletimeout-final.conf "PairableTimeout =
0", it will not emit errors for neither main.conf nor
main.conf.d/10-override-pairabletimeout.conf -- it will only see the
last "PairableTimeout = 0".
So, yes, this is a disadvantage of this approach... it will paper over
problems instead of pinpointing the problem in the individual files.
> So except if I miss something above, I think we should really parse
> each config separately, and not just load and overwrite.
After the explanations above, I guess that you still prefer to load and
parse each file individually, to detect problems in the individual files?
If so, I'll work on it on the next version.
PS: I will be off for some days so please excuse delays in replying.
Cheers.
--
Manuel A. Fernandez Montecelo <mafm@igalia.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: Support for config fragments (conf.d style dirs)
2026-01-15 2:28 [PATCH BlueZ v3 1/1] " Manuel A. Fernandez Montecelo
@ 2026-01-15 3:42 ` bluez.test.bot
0 siblings, 0 replies; 7+ messages in thread
From: bluez.test.bot @ 2026-01-15 3:42 UTC (permalink / raw)
To: linux-bluetooth, mafm
[-- Attachment #1: Type: text/plain, Size: 1262 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=1042572
---Test result---
Test Summary:
CheckPatch PENDING 0.25 seconds
GitLint PENDING 0.27 seconds
BuildEll PASS 20.00 seconds
BluezMake PASS 655.00 seconds
MakeCheck PASS 18.60 seconds
MakeDistcheck PASS 244.55 seconds
CheckValgrind PASS 295.34 seconds
CheckSmatch PASS 349.93 seconds
bluezmakeextell PASS 185.36 seconds
IncrementalBuild PENDING 0.25 seconds
ScanBuild PASS 1052.75 seconds
Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:
##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-01-15 3:42 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-12 20:22 [PATCH BlueZ v2 0/1] Support for config fragments (conf.d style dirs) Manuel A. Fernandez Montecelo
2025-12-12 20:22 ` [PATCH BlueZ v2 1/1] " Manuel A. Fernandez Montecelo
2025-12-12 21:07 ` Luiz Augusto von Dentz
2025-12-12 23:41 ` Manuel A. Fernandez Montecelo
2025-12-12 21:28 ` bluez.test.bot
-- strict thread matches above, loose matches on Subject: below --
2026-01-15 2:28 [PATCH BlueZ v3 1/1] " Manuel A. Fernandez Montecelo
2026-01-15 3:42 ` bluez.test.bot
2025-12-11 21:13 [PATCH BlueZ 1/1] " Manuel A. Fernandez Montecelo
2025-12-11 21:20 ` bluez.test.bot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.