[SSSD] [PATHC] 1625-Confusing error messages for invalid sssd.conf
Dmitri Pal
dpal at redhat.com
Wed Mar 20 16:06:56 UTC 2013
On 03/20/2013 09:16 AM, Jakub Hrozek wrote:
> 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 at 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 at 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?
www.redhat.com/carveoutcosts/
More information about the sssd-devel
mailing list