[SSSD] Re: [PATCH] DEBUG: Add `debug` alias for debug_level

[Petr Cech]

On 04/26/2016 05:08 PM, Stephen Gallagher wrote:
> Our users constantly make the mistake of typing `debug = 9` in the sssd.conf
> instead of `debug_level = 9` as would be correct. This happens frequently-enough
> that we should just alias it rather than continue to have people make mistakes.
>
> Resolves:
> https://fedorahosted.org/sssd/ticket/2999

Hello Stephen,

thank you for your patch.
I see little disruption of our coding style. My comments are inlined.
> 0001-DEBUG-Add-debug-alias-for-debug_level.patch
>
> From f59256f027bb15a5cff317e5b1d418107b4a0a95 Mon Sep 17 00:00:00 2001
> From: Stephen Gallagher 
> Date: Tue, 26 Apr 2016 11:04:36 -0400
> Subject: [PATCH] DEBUG: Add `debug` alias for debug_level
>
> Our users constantly make the mistake of typing `debug = 9` in the
> sssd.conf instead of `debug_level = 9` as would be correct. This
> happens frequently-enough that we should just alias it rather than
> continue to have people make mistakes.
>
> Resolves:
> https://fedorahosted.org/sssd/ticket/2999
>
> src/confdb/confdb.h |  1 +
>  src/util/server.c   | 15 ++++++++++++++-
>  2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
> index a9b1c4362b5c0c6b158830b1bf2ef68db09d8d06..153e68a0761463723945667004b4505acbc5a0b9 100644
> --- a/src/confdb/confdb.h
> +++ b/src/confdb/confdb.h
> @@ -51,10 +51,11 @@
>
> /* Services */
>  #define CONFDB_SERVICE_PATH_TMPL "config/%s"
>  #define CONFDB_SERVICE_COMMAND "command"
>  #define CONFDB_SERVICE_DEBUG_LEVEL "debug_level"
> +#define CONFDB_SERVICE_DEBUG_LEVEL_ALIAS "debug"
>  #define CONFDB_SERVICE_DEBUG_TIMESTAMPS "debug_timestamps"
>  #define CONFDB_SERVICE_DEBUG_MICROSECONDS "debug_microseconds"
>  #define CONFDB_SERVICE_DEBUG_TO_FILES "debug_to_files"
>  #define CONFDB_SERVICE_TIMEOUT "timeout"
>  #define CONFDB_SERVICE_FORCE_TIMEOUT "force_timeout"
> diff --git a/src/util/server.c b/src/util/server.c
> index 67a25955783c30dc03f3d6d9193c8547c625f134..074dc34848ae2b8dd98bf6b218cc6c9c8441104d 100644
> --- a/src/util/server.c
> +++ b/src/util/server.c
> @@ -565,18 +565,31 @@ int server_setup(const char *name, int flags,
>  if (debug_level == SSSDBG_UNRESOLVED) {
>      /* set debug level if any in conf_entry */
>      ret = confdb_get_int(ctx->confdb_ctx, conf_entry,
>                           CONFDB_SERVICE_DEBUG_LEVEL,
>                         SSSDBG_DEFAULT,
>                         SSSDBG_UNRESOLVED,
>                         &debug_level);
>    if (ret != EOK) {
>        DEBUG(SSSDBG_FATAL_FAILURE, "Error reading from confdb (%d) "
>                                     "[%s]\n", ret, strerror(ret));

^ please, remove this space
>          return ret;
>      }
>
>    if (debug_level == SSSDBG_UNRESOLVED) {
>        /* Check for the `debug` alias */
>        ret = confdb_get_int(ctx->confdb_ctx, conf_entry,
>                CONFDB_SERVICE_DEBUG_LEVEL_ALIAS,
>                SSSDBG_DEFAULT,
>                &debug_level);

^ this is the right indentation
>        if (ret != EOK) {
>            DEBUG(SSSDBG_FATAL_FAILURE, "Error reading from confdb (%d) "
>                                        "[%s]\n", ret, strerror(ret));
>            return ret;
>        }
>    }
>    debug_level = debug_convert_old_level(debug_level);
>
> }
>
> /* same for debug timestamps */
>    if (debug_timestamps == SSSDBG_TIMESTAMP_UNRESOLVED) {
>
> -- 2.7.4

The patch looks good to me. So, I will final ACK after:
* CI (hash 0b9529e)
* fixing of coding style.

Regards