On Tue, 2013-11-26 at 14:38 +0100, Jakub Hrozek wrote:
> On Tue, Nov 26, 2013 at 12:29:33PM +0100, Jakub Hrozek wrote:
> > On Sun, Nov 24, 2013 at 08:25:17PM +0100, Pavel Reichl wrote:
> > > On Fri, 2013-11-22 at 16:46 +0100, Jakub Hrozek wrote:
> > > > On Wed, Nov 20, 2013 at 01:52:42PM +0100, Pavel Reichl wrote:
> > > > > New patch hopefully addressing all mentioned issues is attached.
> > > > >
> > > > > PR
> > > > >
> > > >
> > > > Hi Pavel,
> > > >
> > > > sorry for the late review.
> > > >
> > > > The patch works fine and mostly looks good, I'm just wondering about one
> > > > change. I wonder if this block:
> > > >
> > > > > + if (ret == ENOENT) {
> > > > > + /* sss specific error denoting missing configuration
> > > > > file */
> > > > > + ret = ERR_MISSING_CONF;
> > > > > + }
> > > >
> > > > Would be better placed in the confdb_init_db() function after the
> > > > sss_ini_config_file_open() fails with ENOENT? Not 100% sure what would
> > > > look better though.
> > > > _______________________________________________
> > > > sssd-devel mailing list
> > > > sssd-devel@lists.fedorahosted.org
> > > > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> > >
> > > Hi Jakub,
> > >
> > > I think you are right because it makes more obvious which function is
> > > supposed to fail with ENOENT. Improved patch is attached.
> > >
> > > PR
> > >
> >
> > The code works fine and looks good to me.
> >
> > ACK.
> >
> > Congratulations for going through the review process for the first time!
>
> Actually, sorry, one more request for a change..after checking for
> return code of confdb_init_db we need to print the error using
> sss_strerror(). Plain strerror can only print errno codes. Sorry I
> didn't catch this sooner.
> _______________________________________________
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
My bad, thanks for noticing.
>From 391d219e43507ac29843240d5332e67b23ace636 Mon Sep 17 00:00:00 2001
From: Pavel Reichl <pavel.rei...@redhat.com>
Date: Tue, 19 Nov 2013 11:24:31 +0000
Subject: [PATCH] monitor: Specific error message for missing sssd.conf
Specific error message is logged for missing sssd.conf file. New sssd specific
error value is introduced for this case.
Resolves:
https://fedorahosted.org/sssd/ticket/2156
---
src/confdb/confdb_setup.c | 9 +++++++--
src/monitor/monitor.c | 8 +++++++-
src/util/util_errors.c | 1 +
src/util/util_errors.h | 1 +
4 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/src/confdb/confdb_setup.c b/src/confdb/confdb_setup.c
index b13553eaa560bb83ecf7a53b32ab116f38f7f480..2a34e4f7a3e5f9aa37760036520f5274e9289b70 100644
--- a/src/confdb/confdb_setup.c
+++ b/src/confdb/confdb_setup.c
@@ -155,8 +155,13 @@ int confdb_init_db(const char *config_file, struct confdb_ctx *cdb)
/* Open config file */
ret = sss_ini_config_file_open(init_data, config_file);
if (ret != EOK) {
- DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to open configuration file.\n"));
- ret = EIO;
+ DEBUG(SSSDBG_TRACE_FUNC,
+ ("sss_ini_config_file_open failed: %s [%d]\n", strerror(ret),
+ ret));
+ if (ret == ENOENT) {
+ /* sss specific error denoting missing configuration file */
+ ret = ERR_MISSING_CONF;
+ }
goto done;
}
diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index f8a02b8b8be083860a4ae342a27b1297613af996..92bac422a53b450f29a2d5b23217b530030781c2 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -1614,7 +1614,7 @@ static errno_t load_configuration(TALLOC_CTX *mem_ctx,
ret = confdb_init_db(config_file, ctx->cdb);
if (ret != EOK) {
DEBUG(0, ("ConfDB initialization has failed [%s]\n",
- strerror(ret)));
+ sss_strerror(ret)));
goto done;
}
@@ -2785,6 +2785,12 @@ int main(int argc, const char *argv[])
ret = load_configuration(tmp_ctx, config_file, &monitor);
if (ret != EOK) {
switch (ret) {
+ case ERR_MISSING_CONF:
+ DEBUG(SSSDBG_CRIT_FAILURE,
+ ("Configuration file: %s does not exist.\n", config_file));
+ sss_log(SSS_LOG_ALERT,
+ "Configuration file: %s does not exist.\n", config_file);
+ break;
case EPERM:
case EACCES:
DEBUG(SSSDBG_CRIT_FAILURE,
diff --git a/src/util/util_errors.c b/src/util/util_errors.c
index b3d10f97b0567ca21ca08a3f2d326ea401c28aff..114c8b04fd354b166d14e526a3bab6a6c0c05951 100644
--- a/src/util/util_errors.c
+++ b/src/util/util_errors.c
@@ -50,6 +50,7 @@ struct err_string error_to_str[] = {
{ "Dynamic DNS update not possible while offline" }, /* ERR_DYNDNS_OFFLINE */
{ "Entry not found" }, /* ERR_NOT_FOUND */
{ "Domain not found" }, /* ERR_DOMAIN_NOT_FOUND */
+ { "Missing configuration file" }, /* ERR_MISSING_CONF */
};
diff --git a/src/util/util_errors.h b/src/util/util_errors.h
index 685607c5bb1b4e7c37152c876518a2b1c69c18d6..bca45f392b0357c3f1c848768358cb1d47514715 100644
--- a/src/util/util_errors.h
+++ b/src/util/util_errors.h
@@ -72,6 +72,7 @@ enum sssd_errors {
ERR_DYNDNS_OFFLINE,
ERR_NOT_FOUND,
ERR_DOMAIN_NOT_FOUND,
+ ERR_MISSING_CONF,
ERR_LAST /* ALWAYS LAST */
};
--
1.8.3.1
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
