[SSSD] Re: [PATCH] UTILS: Fixing duplication of pid file declaration
On Tue, Aug 09, 2016 at 11:27:12AM +0200, Jakub Hrozek wrote: > On Fri, Aug 05, 2016 at 02:49:59PM +0200, Petr Cech wrote: > > On 08/04/2016 11:06 AM, Jakub Hrozek wrote: > > > On Thu, Aug 04, 2016 at 08:41:34AM +0200, Fabiano Fidêncio wrote: > > > > On Tue, Jul 26, 2016 at 4:01 PM, Petr Cech wrote: > > > > > Hello list, > > > > > > > > > > attached patch fixes duplication of pid file declaration. I hope that > > > > > the > > > > > util/util.h is the right place for it. Another opinion are welcome. > > > > > > > > LGTM! > > > > > > > > I'd wait till someone else reviews the patch, in case you want to be > > > > sure that util/util.h is the right place for the defines. > > > > > > It is, but I don't think that MONITOR_NAME belongs there. I think the > > > pidfile declaration can just use hardcoded sssd (or just #define > > > PIDFILE_BASENAME to "sssd") and MONITOR_NAME should stay in monitor.c > > > > Right, thank you. Fixed patch attached :-) > > ACK > > CI: http://sssd-ci.duckdns.org/logs/job/51/12/summary.html * master: 08cd034c8584b6f058cf565ce66f7f9f7120622f ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] UTILS: Fixing duplication of pid file declaration
On Fri, Aug 05, 2016 at 02:49:59PM +0200, Petr Cech wrote: > On 08/04/2016 11:06 AM, Jakub Hrozek wrote: > > On Thu, Aug 04, 2016 at 08:41:34AM +0200, Fabiano Fidêncio wrote: > > > On Tue, Jul 26, 2016 at 4:01 PM, Petr Cech wrote: > > > > Hello list, > > > > > > > > attached patch fixes duplication of pid file declaration. I hope that > > > > the > > > > util/util.h is the right place for it. Another opinion are welcome. > > > > > > LGTM! > > > > > > I'd wait till someone else reviews the patch, in case you want to be > > > sure that util/util.h is the right place for the defines. > > > > It is, but I don't think that MONITOR_NAME belongs there. I think the > > pidfile declaration can just use hardcoded sssd (or just #define > > PIDFILE_BASENAME to "sssd") and MONITOR_NAME should stay in monitor.c > > Right, thank you. Fixed patch attached :-) ACK CI: http://sssd-ci.duckdns.org/logs/job/51/12/summary.html ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] UTILS: Fixing duplication of pid file declaration
On 08/04/2016 11:06 AM, Jakub Hrozek wrote: On Thu, Aug 04, 2016 at 08:41:34AM +0200, Fabiano Fidêncio wrote: On Tue, Jul 26, 2016 at 4:01 PM, Petr Cech wrote: Hello list, attached patch fixes duplication of pid file declaration. I hope that the util/util.h is the right place for it. Another opinion are welcome. LGTM! I'd wait till someone else reviews the patch, in case you want to be sure that util/util.h is the right place for the defines. It is, but I don't think that MONITOR_NAME belongs there. I think the pidfile declaration can just use hardcoded sssd (or just #define PIDFILE_BASENAME to "sssd") and MONITOR_NAME should stay in monitor.c Right, thank you. Fixed patch attached :-) Regards -- Petr^4 Čech >From 584ed64fb95e5d4e95fa0eb38808f339d2d121ca Mon Sep 17 00:00:00 2001 From: Petr Cech Date: Fri, 5 Aug 2016 14:39:39 +0200 Subject: [PATCH] UTILS: Fixing duplication of pid file declaration Resolves: https://fedorahosted.org/sssd/ticket/2978 --- src/monitor/monitor.c | 3 +-- src/tools/common/sss_process.c | 3 --- src/tools/tools_util.h | 3 --- src/util/util.h| 4 4 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c index 89dd0a91d1fbd41dd853cf8745de732b7059d02b..f154d8e85e01ee70dd9591f83925d4e65c909a7f 100644 --- a/src/monitor/monitor.c +++ b/src/monitor/monitor.c @@ -74,7 +74,6 @@ /* name of the monitor server instance */ #define MONITOR_NAME"sssd" -#define SSSD_PIDFILE_PATH PID_PATH"/"MONITOR_NAME".pid" /* Special value to leave the Kerberos Replay Cache set to use * the libkrb5 defaults @@ -1572,7 +1571,7 @@ static int monitor_cleanup(void) int ret; errno = 0; -ret = unlink(SSSD_PIDFILE_PATH); +ret = unlink(SSSD_PIDFILE); if (ret == -1) { ret = errno; DEBUG(SSSDBG_FATAL_FAILURE, diff --git a/src/tools/common/sss_process.c b/src/tools/common/sss_process.c index d4db4ab6f28fd154b928b77d0c05d4253b6cd9f3..6df260731362641a946f70627217fd5254039233 100644 --- a/src/tools/common/sss_process.c +++ b/src/tools/common/sss_process.c @@ -24,9 +24,6 @@ #include "util/util.h" #include "tools/common/sss_process.h" -#define SSSD_PIDFILE ""PID_PATH"/sssd.pid" -#define MAX_PID_LENGTH 10 - static pid_t parse_pid(const char *strpid) { long value; diff --git a/src/tools/tools_util.h b/src/tools/tools_util.h index 93bc621fa2c98384b22ccd1066f02e9f4db10468..f31e843deb7122ee52664229b75eeb92878abe54 100644 --- a/src/tools/tools_util.h +++ b/src/tools/tools_util.h @@ -27,9 +27,6 @@ #include "util/util.h" -#define SSSD_PIDFILE ""PID_PATH"/sssd.pid" -#define MAX_PID_LENGTH 10 - #define BAD_POPT_PARAMS(pc, msg, val, label) do { \ usage(pc, msg); \ val = EXIT_FAILURE; \ diff --git a/src/util/util.h b/src/util/util.h index 4449315f8b1a79ec915bc340b46188c440a27fa3..9c39a5cc580a94b74a3fd4ea5bd12320714dcfe5 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -53,6 +53,10 @@ #include "util/sss_format.h" #include "util/debug.h" +/* name of the monitor server instance */ +#define SSSD_PIDFILE PID_PATH"/sssd.pid" +#define MAX_PID_LENGTH 10 + #define _(STRING) gettext (STRING) #define ENUM_INDICATOR "*" -- 2.7.4 ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] UTILS: Fixing duplication of pid file declaration
On Thu, Aug 04, 2016 at 08:41:34AM +0200, Fabiano Fidêncio wrote: > On Tue, Jul 26, 2016 at 4:01 PM, Petr Cech wrote: > > Hello list, > > > > attached patch fixes duplication of pid file declaration. I hope that the > > util/util.h is the right place for it. Another opinion are welcome. > > LGTM! > > I'd wait till someone else reviews the patch, in case you want to be > sure that util/util.h is the right place for the defines. It is, but I don't think that MONITOR_NAME belongs there. I think the pidfile declaration can just use hardcoded sssd (or just #define PIDFILE_BASENAME to "sssd") and MONITOR_NAME should stay in monitor.c ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] UTILS: Fixing duplication of pid file declaration
On Tue, Jul 26, 2016 at 4:01 PM, Petr Cech wrote: > Hello list, > > attached patch fixes duplication of pid file declaration. I hope that the > util/util.h is the right place for it. Another opinion are welcome. LGTM! I'd wait till someone else reviews the patch, in case you want to be sure that util/util.h is the right place for the defines. Best Regards, -- Fabiano Fidêncio ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] UTILS: Fixing duplication of pid file declaration
bump -- Petr^4 Čech ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org