On Thu, Nov 29, 2012 at 09:33:08AM -0500, Ariel Barria wrote:
Hi Ariel,
this is a good start!
> From 182c5c70d52734caba59d97ca318b036412a6705 Mon Sep 17 00:00:00 2001
> From: "Ariel O. Barria" <arielb(a)fedoraproject.org>
> Date: Tue, 27 Nov 2012 19:21:05 -0500
> Subject: [PATCH] Confusing error messages for invalid sssd.conf
>
>
https://fedorahosted.org/sssd/ticket/1625
> Amending errors messages and add other error codes to be more specific
> and avoid confusion.
> ---
> src/monitor/monitor.c | 32 ++++++++++++++++++++++++--------
> 1 file changed, 24 insertions(+), 8 deletions(-)
>
> diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
> index
71f915723426e97aaf98c2bccee81a41e7595da8..d9bee5b3b13854841dbaabea19c3db2257df9a48 100644
> --- a/src/monitor/monitor.c
> +++ b/src/monitor/monitor.c
> @@ -2752,15 +2752,31 @@ int main(int argc, const char *argv[])
> /* Parse config file, fail if cannot be done */
> ret = load_configuration(tmp_ctx, config_file, &monitor);
> if (ret != EOK) {
> - if (ret == EPERM) {
> - DEBUG(1, ("Cannot read configuration file %s\n",
config_file));
> + switch (ret) {
> + case EPERM:
> + DEBUG(SSSDBG_CRIT_FAILURE,
> + ("Configuration file %s mode bits too permissive.\n",
> + config_file));
I think we should rather say "Not enough permission to read
configuration file" because maybe EPERM could be returned in case
SELinux denied access as well.
> sss_log(SSS_LOG_ALERT,
> - "Cannot read config file %s, please check if permissions
"
> - "are 0600 and the file is owned by root.root",
config_file);
> - } else {
> - DEBUG(SSSDBG_CRIT_FAILURE, ("Error loading configuration database:
"
> - "[%d]: %s\n", ret,
strerror(ret)));
> - sss_log(SSS_LOG_ALERT, "Cannot load configuration database");
> + "Cannot read config file %s, please check if permissions
"
> + "are 0600 and the file is owned by root.root",
config_file);
> + break;
I think we should only special-case EPERM and not the other error codes
because they are quite generic. I think that for anything else other
than EPERM we should simply say that SSSD couldn't load the
configuration database.
Longer-term, this might be a perfect place to introduce new error codes.
> + case ENOENT:
> + DEBUG(SSSDBG_CRIT_FAILURE, ("Error loading configuration file %s:
"
> + "invalid configuration.\n", config_file));
> + sss_log(SSS_LOG_ALERT, "Cannot load configuration file.");
> + break;
> + case EINVAL:
> + DEBUG(SSSDBG_CRIT_FAILURE, ("Error loading configuration file %s:
"
> + "empty file.\n", config_file));
> + sss_log(SSS_LOG_ALERT, "Cannot load configuration file.");
> + break;
> + default:
> + DEBUG(SSSDBG_CRIT_FAILURE, ("Error loading configuration file
%s.",
> + config_file));
> + sss_log(SSS_LOG_ALERT, "Cannot load configuration file.[%d]:
%s\n",
> + ret, strerror(ret));
> + break;
> }
> return 4;
> }
> --
> 1.7.11.7
>
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Wouldn't this area be affected by the work needed for using newer
version of the libini?
--
Thank you,
Dmitri Pal
Sr. Engineering Manager for IdM portfolio
Red Hat Inc.
-------------------------------
Looking to carve out IT costs?