On Tue, Jul 08, 2014 at 10:23:23AM +0200, Pavel Březina wrote:
On 07/07/2014 02:14 PM, Pavel Reichl wrote:
>On Wed, 2014-07-02 at 13:10 +0200, Pavel Březina wrote:
>>This fixes a bug found by Jan Šafránek. The debug level was not restored
>>after sssd restart because the time of last modification of sssd.conf
>>was older than config.ldb.
>>_______________________________________________
>>sssd-devel mailing list
>>sssd-devel(a)lists.fedorahosted.org
>>https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>
>Hi Pavel,
>
>
>> ret = change_debug_level_tmp(ctx->rctx->cdb, name, type,
arg_new_level);
>>+ if (ret != EOK) {
>>+ goto done;
>>+ }
>>+
>>+ errno = 0;
>
>I just wonder if there is any need to initialize errno as it is set by utime() in
case of failure and we directly check return value of utime().
>I'm not sure but I don't think it's a common practice in SSSD codebase to
always initialize errno.
You are correct sir. I removed it also from sss_debuglevel tool.
Setting errno to 0 is a common practice if you rely on the value of
errno for checks. strtol is one example where you absolutely need to set
errno to 0. Looks like with utime you don't, though..
In general, errno is a poor way of communicating error codes..
>
>I think it would be better to replace this line with comment like you put in commit
message, something like
>
>/* Touch configuration file to make sure debug level is reloaded. */
>
Done.
>>+ if (utime(CONFDB_DEFAULT_CONFIG_FILE, NULL) == -1) {
>>+ ret = errno;
>>+ goto done;
>>+ }
>>+
>>+ ret = EOK;
>
>Thanks,
>
>PR
>
>
>_______________________________________________
>sssd-devel mailing list
>sssd-devel(a)lists.fedorahosted.org
>https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>
Patches are attached.
From 39c19102d7e16a0b0a053c9f8913786533a90bcf Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com>
Date: Wed, 2 Jul 2014 11:14:42 +0200
Subject: [PATCH 1/2] IFP: touch config when changing debug level temporarily
If we want the debug level to switch back to the value from
configuration file we need to touch sssd.conf so it is reloaded.
---
src/responder/ifp/ifp_components.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/src/responder/ifp/ifp_components.c b/src/responder/ifp/ifp_components.c
index e5fa06cdd23fa9dcb3e3601a670a73857e4fc33d..c8ad7673c290658e0c6c6273bce5e62bf88e8383
100644
--- a/src/responder/ifp/ifp_components.c
+++ b/src/responder/ifp/ifp_components.c
@@ -21,6 +21,8 @@
#include <string.h>
#include <talloc.h>
#include <signal.h>
+#include <errno.h>
+#include <utime.h>
#include "config.h"
#include "confdb/confdb.h"
@@ -708,6 +710,17 @@ int ifp_component_change_debug_level_tmp(struct sbus_request
*dbus_req,
}
ret = change_debug_level_tmp(ctx->rctx->cdb, name, type, arg_new_level);
+ if (ret != EOK) {
+ goto done;
+ }
+
+ /* Touch configuration file to make sure debug level is reloaded. */
+ if (utime(CONFDB_DEFAULT_CONFIG_FILE, NULL) == -1) {
+ ret = errno;
+ goto done;
+ }
+
+ ret = EOK;
done:
if (ret != EOK) {
--
1.7.11.7
From d15ad6028aae93560bdfc6e9a37a3af7edbb2431 Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com>
Date: Tue, 8 Jul 2014 10:22:27 +0200
Subject: [PATCH 2/2] sss_debuglevel: remove unnecessary errno
---
src/tools/sss_debuglevel.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/tools/sss_debuglevel.c b/src/tools/sss_debuglevel.c
index c38591cfaf66e67c198bf83ad431df5c3e11e74b..f423733d1134e166dca5cf57dd1dd1e1039deb82
100644
--- a/src/tools/sss_debuglevel.c
+++ b/src/tools/sss_debuglevel.c
@@ -190,7 +190,6 @@ errno_t set_debug_level(struct debuglevel_tool_ctx *tool_ctx,
* Change atime and mtime of sssd.conf,
* so the configuration can be restored on next start.
*/
- errno = 0;
if (utime(config_file, NULL) == -1 ) {
err = errno;
DEBUG(SSSDBG_MINOR_FAILURE, "Unable to change mtime of \"%s\":
%s\n",
--
1.7.11.7
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel