[SSSD] [PATCH]-1414-Improve syslog message when configuration cannot be loaded

Jakub Hrozek jhrozek at redhat.com
Tue Jul 17 20:46:07 UTC 2012


On Mon, Jul 16, 2012 at 09:48:42PM -0500, Ariel Barria wrote:
> 
> New patch attached. 
> 
> > Date: Mon, 16 Jul 2012 12:54:25 +0200
> > From: jhrozek at redhat.com
> > To: sssd-devel at lists.fedorahosted.org
> > Subject: Re: [SSSD] [PATCH]-1414-Improve syslog message when configuration cannot be loaded
> > 
> > On Mon, Jul 16, 2012 at 12:39:06AM -0500, Ariel Barria wrote:
> > > 
> > > https://fedorahosted.org/sssd/ticket/1414
> > > 
> > > I found a similar topic with ./SSSD -i.
> > >  		 	   		  
> > 
> > There is at least one more place that should have been changed by adding
> > strerror which the original reporter complained about, which is the sss_log
> > call at src/monitor/monitor.c:2527

Thank you, Ariel, this is almost what I had in mind. Can you add two more
minor changes? It's mostly coding style, not functional changes. See below.

> -        DEBUG(0,("Could not set up debug fn.\n"));
> +        DEBUG(SSSDBG_FATAL_FAILURE,
> +             ("Could not set up debug fn.\n"));

Statements like this don't have to be wrapped into two lines, if they
fit into 78 characters, it's be OK to leave it on one line.

>          talloc_free(cdb);
>          return EIO;
>      }
> @@ -653,8 +654,10 @@ int confdb_init(TALLOC_CTX *mem_ctx,
>      ret = ldb_connect(cdb->ldb, confdb_location, 0, NULL);
>      umask(old_umask);
>      if (ret != LDB_SUCCESS) {
> -        DEBUG(0, ("Unable to open config database [%s]\n",
> -                  confdb_location));
> +        DEBUG(SSSDBG_FATAL_FAILURE,
> +             ("Unable to open config database"
> +             " [%s], [%d] [%s].\n",
> +              confdb_location, ret, strerror(ret)));

Same here.

>          talloc_free(cdb);
>          return EIO;
>      }
> diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
> index 201d9fa8dd01977ff5455da053fff4c2bd4fcec5..91603310ab060181eb727bc05be199345f9d1b4f 100644
> --- a/src/monitor/monitor.c
> +++ b/src/monitor/monitor.c
> @@ -1381,14 +1381,17 @@ static errno_t load_configuration(TALLOC_CTX *mem_ctx,
>  
>      cdb_file = talloc_asprintf(ctx, "%s/%s", DB_PATH, CONFDB_FILE);
>      if (cdb_file == NULL) {
> -        DEBUG(0,("Out of memory, aborting!\n"));
> +        DEBUG(SSSDBG_FATAL_FAILURE,
> +             ("Out of memory, aborting!.\n"));

Same here.

>          ret = ENOMEM;
>          goto done;
>      }
>  
>      ret = confdb_init(ctx, &ctx->cdb, cdb_file);
>      if (ret != EOK) {
> -        DEBUG(0,("The confdb initialization failed\n"));
> +        DEBUG(SSSDBG_FATAL_FAILURE,
> +             ("The confdb initialization failed, [%d] [%s].\n",
> +             ret,strerror(ret)));

Please put a space after a comma (",").

>          goto done;
>      }
>  
> @@ -1405,25 +1408,31 @@ static errno_t load_configuration(TALLOC_CTX *mem_ctx,
>  
>          ret = confdb_init(ctx, &ctx->cdb, cdb_file);
>          if (ret != EOK) {
> -            DEBUG(0,("The confdb initialization failed\n"));
> +            DEBUG(SSSDBG_FATAL_FAILURE,
> +                 ("The confdb initialization failed, [%d] [%s].\n",
> +                 ret,strerror(ret)));

Space again here.

>              goto done;
>          }
>  
>          /* Load special entries */
>          ret = confdb_create_base(ctx->cdb);
>          if (ret != EOK) {
> -            DEBUG(0, ("Unable to load special entries into confdb\n"));
> +            DEBUG(SSSDBG_FATAL_FAILURE,
> +                 ("Unable to load special entries into confdb.\n"));
>              goto done;
>          }
>      } else if (ret != EOK) {
> -        DEBUG(0, ("Fatal error initializing confdb\n"));
> +        DEBUG(SSSDBG_FATAL_FAILURE,
> +             ("Fatal error initializing confdb, [%d] [%s].\n",
> +             ret,strerror(ret)));

Space again.

>          goto done;
>      }
>      talloc_zfree(cdb_file);
>  
>      ret = confdb_init_db(config_file, ctx->cdb);
>      if (ret != EOK) {
> -        DEBUG(0, ("ConfDB initialization has failed [%s]\n",
> +        DEBUG(SSSDBG_FATAL_FAILURE,
> +             ("ConfDB initialization has failed [%s].\n",
>                strerror(ret)));
>          goto done;
>      }
> @@ -2522,9 +2531,12 @@ int main(int argc, const char *argv[])
>                      "Cannot read config file %s, please check if permissions "
>                      "are 0600 and the file is owned by root.root", config_file);
>          } else {
> -            DEBUG(1, ("Error loading configuration database: [%d]: %s",
> -                      ret, strerror(ret)));
> -            sss_log(SSS_LOG_ALERT, "Cannot load configuration database");
> +            DEBUG(SSSDBG_FATAL_FAILURE,
> +                 ("Error loading configuration database: [%d]: %s",
> +                 ret, strerror(ret)));
> +            sss_log(SSS_LOG_ALERT,
> +                   "Cannot load configuration database: [%d]: %s",
> +                   ret, strerror(ret));
>          }
>          return 4;
>      }
> -- 
> 1.7.7.6
> 



More information about the sssd-devel mailing list