[SSSD] Re: [PATCH] UTILS: Fixing duplication of pid file declaration

2016-08-09 Thread Jakub Hrozek 
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

2016-08-09 Thread Jakub Hrozek 
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

2016-08-05 Thread Petr Cech 

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

2016-08-04 Thread Jakub Hrozek 
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

2016-08-04 Thread Fabiano Fidêncio 
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

2016-08-03 Thread Petr Cech 

bump

--
Petr^4 Čech
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org