On 11/20/2015 06:28 PM, Lukas Slebodnik wrote:
On (20/11/15 16:10), Petr Cech wrote:
> Hi,
>
> there is patch resolving
>
https://fedorahosted.org/sssd/ticket/2808
>
> Regards
>
> Petr
>From 576ca8774302e90af4bc3bdfca73d4f91cef034e Mon Sep 17 00:00:00 2001
> From: Petr Cech <pcech(a)redhat.com>
> Date: Fri, 20 Nov 2015 10:06:56 -0500
> Subject: [PATCH] DEBUG: Add text tag after hexadecimal debug level
>
> This patch adds text tag after debug level in logs.
>
> Resolves:
>
https://fedorahosted.org/sssd/ticket/2808
> ---
> src/tests/debug-tests.c | 15 +++++++------
> src/util/debug.c | 57 +++++++++++++++++++++++++++++++++++++++++++------
> src/util/util.h | 1 +
> 3 files changed, 61 insertions(+), 12 deletions(-)
>
> diff --git a/src/tests/debug-tests.c b/src/tests/debug-tests.c
> index
d904d7eb8b5418608023faca0d62067f3106d23b..bf751788e442dc69dcd8a2d5d6a7b4220f8ef21d 100644
> --- a/src/tests/debug-tests.c
> +++ b/src/tests/debug-tests.c
> @@ -210,10 +210,11 @@ int test_helper_debug_check_message(int level)
> }
> compare_to = talloc_asprintf(ctx,
> "(%s %s %2d %.2d:%.2d:%.2d %.4d)
"
> - "[%s] [%s] (%#.4x): %s\n",
> + "[%s] [%s] (%#.4x | %s): %s\n",
> time_day, time_month, time_day_num,
> time_hour, time_min, time_sec, time_year,
> - debug_prg_name, function, level, body);
> + debug_prg_name, function, level,
> + str_debug_level(level), body);
> if (compare_to == NULL) {
> _errno = ENOMEM;
> ret = DEBUG_TEST_ERROR;
> @@ -230,10 +231,11 @@ int test_helper_debug_check_message(int level)
> }
> compare_to = talloc_asprintf(ctx,
> "(%s %s %2d %.2d:%.2d:%.2d:%.6d %.4d)
"
> - "[%s] [%s] (%#.4x): %s\n",
> + "[%s] [%s] (%#.4x | %s): %s\n",
> time_day, time_month, time_day_num,
> time_hour, time_min, time_sec, time_usec,
> - time_year, debug_prg_name, function, level,
body);
> + time_year, debug_prg_name, function,
level,
> + str_debug_level(level), body);
> if (compare_to == NULL) {
> _errno = ENOMEM;
> ret = DEBUG_TEST_ERROR;
> @@ -241,8 +243,9 @@ int test_helper_debug_check_message(int level)
> }
> }
> } else {
> - compare_to = talloc_asprintf(ctx, "[%s] [%s] (%#.4x): %s\n",
> - debug_prg_name, function, level, body);
> + compare_to = talloc_asprintf(ctx, "[%s] [%s] (%#.4x | %s): %s\n",
> + debug_prg_name, function, level,
> + str_debug_level(level), body);
> if (compare_to == NULL) {
> _errno = ENOMEM;
> ret = DEBUG_TEST_ERROR;
> diff --git a/src/util/debug.c b/src/util/debug.c
> index
a8eea32740155ec3daf6be71ef9a8af6592f74a9..059f38bb568d1eee1eebc05cc57317e21743bde6 100644
> --- a/src/util/debug.c
> +++ b/src/util/debug.c
> @@ -46,6 +46,48 @@ int debug_to_stderr = 0;
> const char *debug_log_file = "sssd";
> FILE *debug_file = NULL;
>
> +const char *str_debug_level(int level) {
This function is not tested.
We need to ensure that new string will be added if
new debug level will be created.
Hi Lukas,
yes, there is no CI test for this function. The SSSDBG_* levels are
constants defined at src/util/util.h. If somebody add something new
there is no way how to trace it. I welcome your advice!
> + const char *message = NULL;
> +
> + switch(level) {
> + case SSSDBG_FATAL_FAILURE:
> + message = "ERR_FATAL_FAILURE";
> + break;
> + case SSSDBG_CRIT_FAILURE:
> + message = "ERR_CRIT_FAILURE";
> + break;
> + case SSSDBG_OP_FAILURE:
> + message = "ERR_OP_FAILURE";
> + break;
> + case SSSDBG_MINOR_FAILURE:
> + message = "ERR_MINOR_FAILURE";
> + break;
> + case SSSDBG_CONF_SETTINGS:
> + message = "INFO_CONF_SETTINGS";
> + break;
> + case SSSDBG_FUNC_DATA:
> + message = "INFO_FUNC_DATA";
> + break;
> + case SSSDBG_TRACE_FUNC:
> + message = "TRACE_FUNC";
> + break;
> + case SSSDBG_TRACE_LIBS:
> + message = "TRACE_LIBS";
> + break;
> + case SSSDBG_TRACE_INTERNAL:
> + message = "TRACE_INTERNAL";
> + break;
> + case SSSDBG_TRACE_ALL:
> + message = "TRACE_ALL";
> + break;
> + case SSSDBG_BE_FO:
> + message = "FAIL_OVER";
> + break;
> + }
Default case could be there, with log message, that there is something
new. Is it enough?
> +
> + return message;
> +}
> +
> errno_t set_debug_file_from_fd(const int fd)
> {
> FILE *dummy;
> @@ -252,18 +294,21 @@ void debug_fn(const char *file,
> memcpy(datetime, ctime(&tv.tv_sec), 19);
> datetime[19] = '\0';
> if (debug_microseconds) {
> - debug_printf("(%s:%.6ld %d) [%s] [%s] (%#.4x): ",
> + debug_printf("(%s:%.6ld %d) [%s] [%s] (%#.4x | %s): ",
^^^^^^^^^^^^
This change would complicate
filtering of messages based on hexadecmal number.
The expectation was that debug level was between
round brackets.
Right. I can rewrite it to form
(%#.4x) (%s)
And I need to say I am not still happy with that. There is another
thread on list about it.
LS
Petr
> _______________________________________________
> sssd-devel mailing list
> sssd-devel(a)lists.fedorahosted.org
>
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
>