9:46 a.m.
In the attached patch I've attempted to clean up the log levels used a
little bit. I am not saying these are all the changes that could/should
be done but we can start with this patch. So please review only these
changes for now and additional changes can be done in further patches.
I tried to follow the MWG - memory allocation errors should be LOG_CRIT
and errors induced by the authenticating user should be LOG_NOTICE. For
the rest I just tried to somehow make them more reasonable. Mostly I am
lowering the level but there are some cases where the level is raised
too.
It also addresses the https://fedorahosted.org/linux-pam/ticket/63
Please review,
--
Tomas Mraz
No matter how far down the wrong road you've gone, turn back.
Turkish proverb
(You'll never know whether the road is wrong though.)
10:14 a.m.
On St, 2016-06-15 at 16:46 +0200, Tomas Mraz wrote:
In the attached patch I've attempted to clean up the log levels
used
a
little bit. I am not saying these are all the changes that
could/should
be done but we can start with this patch. So please review only these
changes for now and additional changes can be done in further
patches.
I tried to follow the MWG - memory allocation errors should be
LOG_CRIT
and errors induced by the authenticating user should be LOG_NOTICE.
For
the rest I just tried to somehow make them more reasonable. Mostly I
am
lowering the level but there are some cases where the level is raised
too.
It also addresses the https://fedorahosted.org/linux-pam/ticket/63
Please review,
Ping? Any comments?
--
Tomas Mraz
No matter how far down the wrong road you've gone, turn back.
Turkish proverb
(You'll never know whether the road is wrong though.)
4:27 a.m.
On Po, 2016-06-20 at 17:14 +0200, Tomas Mraz wrote:
On St, 2016-06-15 at 16:46 +0200, Tomas Mraz wrote:
>
> In the attached patch I've attempted to clean up the log levels
> used
> a
> little bit. I am not saying these are all the changes that
> could/should
> be done but we can start with this patch. So please review only
> these
> changes for now and additional changes can be done in further
> patches.
>
> I tried to follow the MWG - memory allocation errors should be
> LOG_CRIT
> and errors induced by the authenticating user should be LOG_NOTICE.
> For
> the rest I just tried to somehow make them more reasonable. Mostly
> I
> am
> lowering the level but there are some cases where the level is
> raised
> too.
>
> It also addresses the https://fedorahosted.org/linux-pam/ticket/63
>
> Please review,
Ping? Any comments?
If there is no comment by Friday, I'll commit this.
--
Tomas Mraz
No matter how far down the wrong road you've gone, turn back.
Turkish proverb
(You'll never know whether the road is wrong though.)
7:06 a.m.
Hi,
sorry for beeing late, was on conferences and didn't found the time
to answer...
On Wed, Jun 15, Tomas Mraz wrote:
diff --git a/modules/pam_access/pam_access.c
b/modules/pam_access/pam_access.c
index 3ac1ad0..f3ea35a 100644
--- a/modules/pam_access/pam_access.c
+++ b/modules/pam_access/pam_access.c
@@ -402,7 +402,7 @@ login_access (pam_handle_t *pamh, struct login_info *item)
(void) fclose(fp);
} else if (errno == ENOENT) {
/* This is no error. */
- pam_syslog(pamh, LOG_WARNING, "warning: cannot open %s: %m",
+ pam_syslog(pamh, LOG_INFO, "warning: cannot open %s: %m",
item->config_file);
} else {
pam_syslog(pamh, LOG_ERR, "cannot open %s: %m", item->config_file);
I disagree here. The message is warning, so the log level should not
be info. Or the message should be adjusted.
But, if a user configures pam_access to use a configuration file, and this
file does not exist, this should be more than LOG_INFO. That's not a
message which is printed regular, it's only printed if the user made
a mistake in the configuration. So I wouldn't change this priority.
Else I'm fine with the changes.
Thorsten
--
Thorsten Kukuk, Senior Architect SLES & Common Code Base
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nuernberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
7:27 a.m.
On Čt, 2016-06-30 at 14:06 +0200, Thorsten Kukuk wrote:
Hi,
sorry for beeing late, was on conferences and didn't found the time
to answer...
On Wed, Jun 15, Tomas Mraz wrote:
>
> diff --git a/modules/pam_access/pam_access.c
> b/modules/pam_access/pam_access.c
> index 3ac1ad0..f3ea35a 100644
> --- a/modules/pam_access/pam_access.c
> +++ b/modules/pam_access/pam_access.c
> @@ -402,7 +402,7 @@ login_access (pam_handle_t *pamh, struct
> login_info *item)
> (void) fclose(fp);
> } else if (errno == ENOENT) {
> /* This is no error. */
> - pam_syslog(pamh, LOG_WARNING, "warning: cannot open %s:
> %m",
> + pam_syslog(pamh, LOG_INFO, "warning: cannot open %s: %m",
> item->config_file);
> } else {
> pam_syslog(pamh, LOG_ERR, "cannot open %s: %m", item-
> >config_file);
I disagree here. The message is warning, so the log level should not
be info. Or the message should be adjusted.
But, if a user configures pam_access to use a configuration file, and
this
file does not exist, this should be more than LOG_INFO. That's not a
message which is printed regular, it's only printed if the user made
a mistake in the configuration. So I wouldn't change this priority.
OK, that makes sense, I will leave it as is.
--
Tomas Mraz
No matter how far down the wrong road you've gone, turn back.
Turkish proverb
(You'll never know whether the road is wrong though.)
2851
days inactive
2866
days old
pam-developers@lists.fedorahosted.org
4 comments
2 participants
participants (2)
-
Thorsten Kukuk -
Tomas Mraz