| Message ID | 20220323000654.3157833-1-luiz.dentz@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [BlueZ,v2,1/9] log: Introduce DBG_IS_ENABLED | expand |
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=625602
---Test result---
Test Summary:
CheckPatch FAIL 13.01 seconds
GitLint PASS 8.92 seconds
Prep - Setup ELL PASS 43.75 seconds
Build - Prep PASS 0.71 seconds
Build - Configure PASS 8.81 seconds
Build - Make PASS 1290.37 seconds
Make Check PASS 11.94 seconds
Make Check w/Valgrind PASS 439.98 seconds
Make Distcheck PASS 232.10 seconds
Build w/ext ELL - Configure PASS 8.79 seconds
Build w/ext ELL - Make PASS 1255.66 seconds
Incremental Build with patchesPASS 11577.93 seconds
Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script with rule in .checkpatch.conf
Output:
[BlueZ,v2,3/9] mgmt: Introduce mgmt_set_verbose
WARNING:REPEATED_WORD: Possible repeated word: 'the'
#83:
the the likes hexdump of packets, by default it is disabled since in
/github/workspace/src/12789344.patch total: 0 errors, 1 warnings, 61 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/12789344.patch has style problems, please review.
NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
---
Regards,
Linux Bluetooth
Hi Luiz, > mgmt_debug callback is used to print debug strings from mgmt instances > which includes the file and function names so using DBG would add yet > another set of file and function prefixes which makes the logs > confusing. > --- > src/adapter.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/src/adapter.c b/src/adapter.c > index 97ce26f8e..6680c5410 100644 > --- a/src/adapter.c > +++ b/src/adapter.c > @@ -10327,9 +10327,8 @@ static void read_version_complete(uint8_t status, uint16_t length, > > static void mgmt_debug(const char *str, void *user_data) > { > - const char *prefix = user_data; > - > - info("%s%s", prefix, str); > + if (DBG_IS_ENABLED()) > + btd_debug(0xffff, "%s", str); > } > > int adapter_init(void) > @@ -10342,8 +10341,7 @@ int adapter_init(void) > return -EIO; > } > > - if (getenv("MGMT_DEBUG")) > - mgmt_set_debug(mgmt_primary, mgmt_debug, "mgmt: ", NULL); > + mgmt_set_debug(mgmt_primary, mgmt_debug, NULL, NULL); oh no. This is crazy. Please re-think this and what computational overhead you are introducing. Regards Marcel
Hi Luiz, >>> mgmt_debug callback is used to print debug strings from mgmt instances >>> which includes the file and function names so using DBG would add yet >>> another set of file and function prefixes which makes the logs >>> confusing. >>> --- >>> src/adapter.c | 8 +++----- >>> 1 file changed, 3 insertions(+), 5 deletions(-) >>> >>> diff --git a/src/adapter.c b/src/adapter.c >>> index 97ce26f8e..6680c5410 100644 >>> --- a/src/adapter.c >>> +++ b/src/adapter.c >>> @@ -10327,9 +10327,8 @@ static void read_version_complete(uint8_t status, uint16_t length, >>> >>> static void mgmt_debug(const char *str, void *user_data) >>> { >>> - const char *prefix = user_data; >>> - >>> - info("%s%s", prefix, str); >>> + if (DBG_IS_ENABLED()) >>> + btd_debug(0xffff, "%s", str); >>> } >>> >>> int adapter_init(void) >>> @@ -10342,8 +10341,7 @@ int adapter_init(void) >>> return -EIO; >>> } >>> >>> - if (getenv("MGMT_DEBUG")) >>> - mgmt_set_debug(mgmt_primary, mgmt_debug, "mgmt: ", NULL); >>> + mgmt_set_debug(mgmt_primary, mgmt_debug, NULL, NULL); >> >> oh no. This is crazy. Please re-think this and what computational overhead you are introducing. > > I considered moving DBG_IS_ENABLED() in place of getenv("MGMT_DEBUG") > so that would be use just once per adapter, the problem is that > wouldn't work with: why do you need this in the first place. The mgmt protocol is also added to btmon traces. Unless you work on src/shared/mgmt.c directly, you don’t need to the debug feature at all. Just let btmon decode it for you. Regards Marcel
diff --git a/src/log.c b/src/log.c index 0155a6bba..1157859ef 100644 --- a/src/log.c +++ b/src/log.c @@ -179,6 +179,18 @@ void __btd_log_init(const char *debug, int detach) info("Bluetooth daemon %s", VERSION); } +bool __btd_log_is_enabled(const char *file) +{ + struct btd_debug_desc *desc; + + for (desc = __start___debug; desc < __stop___debug; desc++) { + if (desc->file && g_pattern_match_simple(file, desc->file)) + return desc->flags & BTD_DEBUG_FLAG_PRINT; + } + + return false; +} + void __btd_log_cleanup(void) { closelog(); diff --git a/src/log.h b/src/log.h index 74941beb2..e35238870 100644 --- a/src/log.h +++ b/src/log.h @@ -9,6 +9,7 @@ */ #include <stdint.h> +#include <stdbool.h> void info(const char *format, ...) __attribute__((format(printf, 1, 2))); @@ -27,6 +28,7 @@ void btd_debug(uint16_t index, const char *format, ...) void __btd_log_init(const char *debug, int detach); void __btd_log_cleanup(void); void __btd_toggle_debug(void); +bool __btd_log_is_enabled(const char *file); struct btd_debug_desc { const char *file; @@ -38,6 +40,15 @@ struct btd_debug_desc { void __btd_enable_debug(struct btd_debug_desc *start, struct btd_debug_desc *stop); +/* DBG_IS_ENABLED: + * + * Simple macro that can be used to check if debug has been enabled for the + * __FILE__. + * Note: This does a lookup thus why it was not used by the likes of + * DBG/DBG_IDX which loads it directly from section("__debug"). + */ +#define DBG_IS_ENABLED() __btd_log_is_enabled(__FILE__) + /** * DBG: * @fmt: format string
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> This introduces DBG_IS_ENABLE macro which can be used to check if BTD_DEBUG_FLAG_PRINT has been enabled for the current file. --- src/log.c | 12 ++++++++++++ src/log.h | 11 +++++++++++ 2 files changed, 23 insertions(+)