Date: Tue, 17 Jul 2012 22:46:07 +0200
From: jhrozek(a)redhat.com
To: sssd-devel(a)lists.fedorahosted.org
Subject: Re: [SSSD] [PATCH]-1414-Improve syslog message when configuration cannot be
loaded
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(a)redhat.com
> > To: sssd-devel(a)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
>
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel