Re: [devel] [PATCH 1/2] base: Add support to direct OpenSAF logging to local node file [#2306]

2018-04-19 Thread Minh Hon Chau

Hi all,


V3 has been sent for review, it's basically not many different to V2, it 
mainly moves the TraceLog to a separate file as Hans' suggestion.



Regards,

Minh


On 19/04/18 15:28, Minh Hon Chau wrote:


Hi Hans,


That's why it needs explicitly "extern C++", I have changed it as your 
suggestion


I have sent out V2, still keep TraceLog in logtrace.h as of now, so it 
can be included once for either c or c++.


Let's see from Anders and Ravi if we have any ideas, I'm happy to move 
it out of logtrace.h too, maybe the existing class Trace as well



Regards,

Minh



On 19/04/18 06:47, Hans Nordebäck wrote:


Hi Minh,


One part of this problem is in mbcsv_papi.h, the

#ifdef __cplusplus

extern "C" {

should at least be after the  #include stmts. Fixing this should 
remove the need for the extern "C++". But moving this part to


a separate header file would also be good.


As a general comment, (all OpenSAF), is that only extern "C" should 
be needed, and it should


be carefully placed only where it is required, c++ access to a c 
function/variable with external linkage, not placing


it in a too wide scope. It can be placed around a group of 
c-functions but not the complete header file.


/Thanks HansN


*Från:* Hans Nordebäck <hans.nordeb...@ericsson.com>
*Skickat:* den 18 april 2018 20:43:22
*Till:* Minh Hon Chau; Anders Widell; ravisekhar.ko...@oracle.com
*Kopia:* opensaf-devel@lists.sourceforge.net
*Ämne:* Re: [devel] [PATCH 1/2] base: Add support to direct OpenSAF 
logging to local node file [#2306]

Hi Minh,


yes, before this patch logtrace.h was a c header file callable from c 
and c++. Now it is a c/c++ header file


so including it from a c program without the extern "c++" will fail. 
In the first review comment I suggested


to move this part to a separate header file and keep logtrace.h as 
before.


/Regards HansN


Från: Minh Hon Chau <minh.c...@dektech.com.au>
Skickat: den 18 april 2018 16:09:35
Till: Hans Nordebäck; Anders Widell; ravisekhar.ko...@oracle.com
Kopia: opensaf-devel@lists.sourceforge.net
Ämne: Re: [PATCH 1/2] base: Add support to direct OpenSAF logging to 
local node file [#2306]


Hi Hans,

One comment regarding extern C++ as below

Thanks,

Minh


On 18/04/18 23:37, Hans Nordebäck wrote:
> Hi Minh,
>
>   See my comments below.
>
> /Thanks HansN
>
> -Original Message-
> From: Minh Hon Chau [mailto:minh.c...@dektech.com.au]
> Sent: den 18 april 2018 15:20
> To: Hans Nordebäck <hans.nordeb...@ericsson.com>; Anders Widell 
<anders.wid...@ericsson.com>; ravisekhar.ko...@oracle.com

> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [PATCH 1/2] base: Add support to direct OpenSAF 
logging to local node file [#2306]

>
> Hi Hans,
>
> Please check my response with [Minh]
>
> Thanks
>
> Minh
>
>
> On 18/04/18 22:40, Hans Nordeback wrote:
>> Hi Minh,
>>
>> ack, code review only. Some comments below.
>>
>> /Thanks HansN
>>
>>
>> On 04/12/2018 01:12 AM, Minh Chau wrote:
>>> Unify TraceLog and MdsLog class to one class (TraceLog) so it can be
>>> used as a common log client.
>>> Add new instance TraceLog for OpenSAF logging to local file, which
>>> can be enabled/disabled via environment variable OSAF_LOCAL_NODE_LOG
>>> ---
>>>    src/base/logtrace.cc | 167
>>> ++-
>>>    src/base/logtrace.h  |  50 +--
>>>    src/mds/mds_log.cc   | 114 +++
>>>    3 files changed, 140 insertions(+), 191 deletions(-)
>>>
>>> diff --git a/src/base/logtrace.cc b/src/base/logtrace.cc index
>>> b046fab..857e31c 100644
>>> --- a/src/base/logtrace.cc
>>> +++ b/src/base/logtrace.cc
>>> @@ -36,15 +36,10 @@
>>>    #include 
>>>    #include 
>>>    #include 
>>> -#include "base/buffer.h"
>>> -#include "base/conf.h"
>>> -#include "base/log_message.h"
>>> -#include "base/macros.h"
>>> -#include "base/mutex.h"
>>> +#include "base/getenv.h"
>>>    #include "base/ncsgl_defs.h"
>>>    #include "base/osaf_utility.h"
>>>    #include "base/time.h"
>>> -#include "base/unix_client_socket.h"
>>>    #include "dtm/common/osaflog_protocol.h"
>>>  namespace global {
>>> @@ -55,65 +50,38 @@ const char *const prefix_name[] = {"EM", "AL",
>>> "CR", "ER", "WA", "NO"

Re: [devel] [PATCH 1/2] base: Add support to direct OpenSAF logging to local node file [#2306]

2018-04-18 Thread Minh Hon Chau

Hi Hans,


That's why it needs explicitly "extern C++", I have changed it as your 
suggestion


I have sent out V2, still keep TraceLog in logtrace.h as of now, so it 
can be included once for either c or c++.


Let's see from Anders and Ravi if we have any ideas, I'm happy to move 
it out of logtrace.h too, maybe the existing class Trace as well



Regards,

Minh



On 19/04/18 06:47, Hans Nordebäck wrote:


Hi Minh,


One part of this problem is in mbcsv_papi.h, the

#ifdef __cplusplus

extern "C" {

should at least be after the  #include stmts. Fixing this should 
remove the need for the extern "C++". But moving this part to


a separate header file would also be good.


As a general comment, (all OpenSAF), is that only extern "C" should be 
needed, and it should


be carefully placed only where it is required, c++ access to a c 
function/variable with external linkage, not placing


it in a too wide scope. It can be placed around a group of c-functions 
but not the complete header file.


/Thanks HansN


*Från:* Hans Nordebäck <hans.nordeb...@ericsson.com>
*Skickat:* den 18 april 2018 20:43:22
*Till:* Minh Hon Chau; Anders Widell; ravisekhar.ko...@oracle.com
*Kopia:* opensaf-devel@lists.sourceforge.net
*Ämne:* Re: [devel] [PATCH 1/2] base: Add support to direct OpenSAF 
logging to local node file [#2306]

Hi Minh,


yes, before this patch logtrace.h was a c header file callable from c 
and c++. Now it is a c/c++ header file


so including it from a c program without the extern "c++" will fail. 
In the first review comment I suggested


to move this part to a separate header file and keep logtrace.h as before.

/Regards HansN


Från: Minh Hon Chau <minh.c...@dektech.com.au>
Skickat: den 18 april 2018 16:09:35
Till: Hans Nordebäck; Anders Widell; ravisekhar.ko...@oracle.com
Kopia: opensaf-devel@lists.sourceforge.net
Ämne: Re: [PATCH 1/2] base: Add support to direct OpenSAF logging to 
local node file [#2306]


Hi Hans,

One comment regarding extern C++ as below

Thanks,

Minh


On 18/04/18 23:37, Hans Nordebäck wrote:
> Hi Minh,
>
>   See my comments below.
>
> /Thanks HansN
>
> -Original Message-
> From: Minh Hon Chau [mailto:minh.c...@dektech.com.au]
> Sent: den 18 april 2018 15:20
> To: Hans Nordebäck <hans.nordeb...@ericsson.com>; Anders Widell 
<anders.wid...@ericsson.com>; ravisekhar.ko...@oracle.com

> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [PATCH 1/2] base: Add support to direct OpenSAF logging 
to local node file [#2306]

>
> Hi Hans,
>
> Please check my response with [Minh]
>
> Thanks
>
> Minh
>
>
> On 18/04/18 22:40, Hans Nordeback wrote:
>> Hi Minh,
>>
>> ack, code review only. Some comments below.
>>
>> /Thanks HansN
>>
>>
>> On 04/12/2018 01:12 AM, Minh Chau wrote:
>>> Unify TraceLog and MdsLog class to one class (TraceLog) so it can be
>>> used as a common log client.
>>> Add new instance TraceLog for OpenSAF logging to local file, which
>>> can be enabled/disabled via environment variable OSAF_LOCAL_NODE_LOG
>>> ---
>>>    src/base/logtrace.cc | 167
>>> ++-
>>>    src/base/logtrace.h  |  50 +--
>>>    src/mds/mds_log.cc   | 114 +++
>>>    3 files changed, 140 insertions(+), 191 deletions(-)
>>>
>>> diff --git a/src/base/logtrace.cc b/src/base/logtrace.cc index
>>> b046fab..857e31c 100644
>>> --- a/src/base/logtrace.cc
>>> +++ b/src/base/logtrace.cc
>>> @@ -36,15 +36,10 @@
>>>    #include 
>>>    #include 
>>>    #include 
>>> -#include "base/buffer.h"
>>> -#include "base/conf.h"
>>> -#include "base/log_message.h"
>>> -#include "base/macros.h"
>>> -#include "base/mutex.h"
>>> +#include "base/getenv.h"
>>>    #include "base/ncsgl_defs.h"
>>>    #include "base/osaf_utility.h"
>>>    #include "base/time.h"
>>> -#include "base/unix_client_socket.h"
>>>    #include "dtm/common/osaflog_protocol.h"
>>>  namespace global {
>>> @@ -55,65 +50,38 @@ const char *const prefix_name[] = {"EM", "AL",
>>> "CR", "ER", "WA", "NO", "IN",
>>>   "T6", "T7", "T8", ">>", "<<"};
>>>    char *msg_id;
>>>    int log

Re: [devel] [PATCH 1/2] base: Add support to direct OpenSAF logging to local node file [#2306]

2018-04-18 Thread Hans Nordebäck
Hi Minh,


One part of this problem is in mbcsv_papi.h, the

#ifdef __cplusplus

extern "C" {

should at least be after the  #include stmts. Fixing this should remove the 
need for the extern "C++". But moving this part to

a separate header file would also be good.


As a general comment, (all OpenSAF), is that only extern "C" should be needed, 
and it should

be carefully placed only where it is required, c++ access to a c 
function/variable with external linkage, not placing

it in a too wide scope. It can be placed around a group of c-functions but not 
the complete header file.

/Thanks HansN


Från: Hans Nordebäck <hans.nordeb...@ericsson.com>
Skickat: den 18 april 2018 20:43:22
Till: Minh Hon Chau; Anders Widell; ravisekhar.ko...@oracle.com
Kopia: opensaf-devel@lists.sourceforge.net
Ämne: Re: [devel] [PATCH 1/2] base: Add support to direct OpenSAF logging to 
local node file [#2306]

Hi Minh,


yes, before this patch logtrace.h was a c header file callable from c and c++. 
Now it is a c/c++ header file

so including it from a c program without the extern "c++" will fail. In the 
first review comment I suggested

to move this part to a separate header file and keep logtrace.h as before.

/Regards HansN


Från: Minh Hon Chau <minh.c...@dektech.com.au>
Skickat: den 18 april 2018 16:09:35
Till: Hans Nordebäck; Anders Widell; ravisekhar.ko...@oracle.com
Kopia: opensaf-devel@lists.sourceforge.net
Ämne: Re: [PATCH 1/2] base: Add support to direct OpenSAF logging to local node 
file [#2306]

Hi Hans,

One comment regarding extern C++ as below

Thanks,

Minh


On 18/04/18 23:37, Hans Nordebäck wrote:
> Hi Minh,
>
>   See my comments below.
>
> /Thanks HansN
>
> -Original Message-
> From: Minh Hon Chau [mailto:minh.c...@dektech.com.au]
> Sent: den 18 april 2018 15:20
> To: Hans Nordebäck <hans.nordeb...@ericsson.com>; Anders Widell 
> <anders.wid...@ericsson.com>; ravisekhar.ko...@oracle.com
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [PATCH 1/2] base: Add support to direct OpenSAF logging to local 
> node file [#2306]
>
> Hi Hans,
>
> Please check my response with [Minh]
>
> Thanks
>
> Minh
>
>
> On 18/04/18 22:40, Hans Nordeback wrote:
>> Hi Minh,
>>
>> ack, code review only. Some comments below.
>>
>> /Thanks HansN
>>
>>
>> On 04/12/2018 01:12 AM, Minh Chau wrote:
>>> Unify TraceLog and MdsLog class to one class (TraceLog) so it can be
>>> used as a common log client.
>>> Add new instance TraceLog for OpenSAF logging to local file, which
>>> can be enabled/disabled via environment variable OSAF_LOCAL_NODE_LOG
>>> ---
>>>src/base/logtrace.cc | 167
>>> ++-
>>>src/base/logtrace.h  |  50 +--
>>>src/mds/mds_log.cc   | 114 +++
>>>3 files changed, 140 insertions(+), 191 deletions(-)
>>>
>>> diff --git a/src/base/logtrace.cc b/src/base/logtrace.cc index
>>> b046fab..857e31c 100644
>>> --- a/src/base/logtrace.cc
>>> +++ b/src/base/logtrace.cc
>>> @@ -36,15 +36,10 @@
>>>#include 
>>>#include 
>>>#include 
>>> -#include "base/buffer.h"
>>> -#include "base/conf.h"
>>> -#include "base/log_message.h"
>>> -#include "base/macros.h"
>>> -#include "base/mutex.h"
>>> +#include "base/getenv.h"
>>>#include "base/ncsgl_defs.h"
>>>#include "base/osaf_utility.h"
>>>#include "base/time.h"
>>> -#include "base/unix_client_socket.h"
>>>#include "dtm/common/osaflog_protocol.h"
>>>  namespace global {
>>> @@ -55,65 +50,38 @@ const char *const prefix_name[] = {"EM", "AL",
>>> "CR", "ER", "WA", "NO", "IN",
>>>   "T6", "T7", "T8", ">>", "<<"};
>>>char *msg_id;
>>>int logmask;
>>> +const char* osaf_log_file = "osaf.log"; bool enable_osaf_log =
>>> +false;
>>>  }  // namespace global
>>>-class TraceLog {
>>> - public:
>>> -  static bool Init();
>>> -  static void Log(base::LogMessage::Severity severity, const char
>>> *fmt,
>>> -  va_list ap);
>>> -
>>> - private:
>>> -  TraceLog(const std::strin

Re: [devel] [PATCH 1/2] base: Add support to direct OpenSAF logging to local node file [#2306]

2018-04-18 Thread Hans Nordebäck
Hi Minh,


yes, before this patch logtrace.h was a c header file callable from c and c++. 
Now it is a c/c++ header file

so including it from a c program without the extern "c++" will fail. In the 
first review comment I suggested

to move this part to a separate header file and keep logtrace.h as before.

/Regards HansN


Från: Minh Hon Chau 
Skickat: den 18 april 2018 16:09:35
Till: Hans Nordebäck; Anders Widell; ravisekhar.ko...@oracle.com
Kopia: opensaf-devel@lists.sourceforge.net
Ämne: Re: [PATCH 1/2] base: Add support to direct OpenSAF logging to local node 
file [#2306]

Hi Hans,

One comment regarding extern C++ as below

Thanks,

Minh


On 18/04/18 23:37, Hans Nordebäck wrote:
> Hi Minh,
>
>   See my comments below.
>
> /Thanks HansN
>
> -Original Message-
> From: Minh Hon Chau [mailto:minh.c...@dektech.com.au]
> Sent: den 18 april 2018 15:20
> To: Hans Nordebäck ; Anders Widell 
> ; ravisekhar.ko...@oracle.com
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [PATCH 1/2] base: Add support to direct OpenSAF logging to local 
> node file [#2306]
>
> Hi Hans,
>
> Please check my response with [Minh]
>
> Thanks
>
> Minh
>
>
> On 18/04/18 22:40, Hans Nordeback wrote:
>> Hi Minh,
>>
>> ack, code review only. Some comments below.
>>
>> /Thanks HansN
>>
>>
>> On 04/12/2018 01:12 AM, Minh Chau wrote:
>>> Unify TraceLog and MdsLog class to one class (TraceLog) so it can be
>>> used as a common log client.
>>> Add new instance TraceLog for OpenSAF logging to local file, which
>>> can be enabled/disabled via environment variable OSAF_LOCAL_NODE_LOG
>>> ---
>>>src/base/logtrace.cc | 167
>>> ++-
>>>src/base/logtrace.h  |  50 +--
>>>src/mds/mds_log.cc   | 114 +++
>>>3 files changed, 140 insertions(+), 191 deletions(-)
>>>
>>> diff --git a/src/base/logtrace.cc b/src/base/logtrace.cc index
>>> b046fab..857e31c 100644
>>> --- a/src/base/logtrace.cc
>>> +++ b/src/base/logtrace.cc
>>> @@ -36,15 +36,10 @@
>>>#include 
>>>#include 
>>>#include 
>>> -#include "base/buffer.h"
>>> -#include "base/conf.h"
>>> -#include "base/log_message.h"
>>> -#include "base/macros.h"
>>> -#include "base/mutex.h"
>>> +#include "base/getenv.h"
>>>#include "base/ncsgl_defs.h"
>>>#include "base/osaf_utility.h"
>>>#include "base/time.h"
>>> -#include "base/unix_client_socket.h"
>>>#include "dtm/common/osaflog_protocol.h"
>>>  namespace global {
>>> @@ -55,65 +50,38 @@ const char *const prefix_name[] = {"EM", "AL",
>>> "CR", "ER", "WA", "NO", "IN",
>>>   "T6", "T7", "T8", ">>", "<<"};
>>>char *msg_id;
>>>int logmask;
>>> +const char* osaf_log_file = "osaf.log"; bool enable_osaf_log =
>>> +false;
>>>  }  // namespace global
>>>-class TraceLog {
>>> - public:
>>> -  static bool Init();
>>> -  static void Log(base::LogMessage::Severity severity, const char
>>> *fmt,
>>> -  va_list ap);
>>> -
>>> - private:
>>> -  TraceLog(const std::string , const std::string _name,
>>> -   uint32_t proc_id, const std::string _id,
>>> -   const std::string _name);
>>> -  void LogInternal(base::LogMessage::Severity severity, const char
>>> *fmt,
>>> -   va_list ap);
>>> -  static constexpr const uint32_t kMaxSequenceId =
>>> uint32_t{0x7fff};
>>> -  static TraceLog *instance_;
>>> -  const base::LogMessage::HostName fqdn_;
>>> -  const base::LogMessage::AppName app_name_;
>>> -  const base::LogMessage::ProcId proc_id_;
>>> -  const base::LogMessage::MsgId msg_id_;
>>> -  uint32_t sequence_id_;
>>> -  base::UnixClientSocket log_socket_;
>>> -  base::Buffer<512> buffer_;
>>> -  base::Mutex mutex_;
>>> -
>>> -  DELETE_COPY_AND_MOVE_OPERATORS(TraceLog);
>>> -};
>>> -
>>> -TraceLog *TraceLog::instance_ = nullptr;
>>> -
>>> -TraceLog::TraceLog(const std::string , const std::string
>>> _name,
>>> -   uint32_t proc_id, const std::string _id,
>>> -   const std::string _name)
>>> -: fqdn_{base::LogMessage::HostName{fqdn}},
>>> -  app_name_{base::LogMessage::AppName{app_name}},
>>> - proc_id_{base::LogMessage::ProcId{std::to_string(proc_id)}},
>>> -  msg_id_{base::LogMessage::MsgId{msg_id}},
>>> -  sequence_id_{1},
>>> -  log_socket_{socket_name, base::UnixSocket::kBlocking},
>>> -  buffer_{},
>>> -  mutex_{} {}
>>> -
>>> -bool TraceLog::Init() {
>>> -  if (instance_ != nullptr) return false;
>>> -  char app_name[49];
>>> -  char pid_path[1024];
>> [HansN] instead of static global use unnamed namespaces instead. Also
>> try to avoid globals, why change Log and Init from static members? (An
>> alternative is to use a singleton instead, if needed)
> [Minh] Changing Log() and Init() not to be static because I am not using 
> singleton any more. 

Re: [devel] [PATCH 1/2] base: Add support to direct OpenSAF logging to local node file [#2306]

2018-04-18 Thread Minh Hon Chau

Hi Hans,

One comment regarding extern C++ as below

Thanks,

Minh


On 18/04/18 23:37, Hans Nordebäck wrote:

Hi Minh,

  See my comments below.

/Thanks HansN

-Original Message-
From: Minh Hon Chau [mailto:minh.c...@dektech.com.au]
Sent: den 18 april 2018 15:20
To: Hans Nordebäck ; Anders Widell 
; ravisekhar.ko...@oracle.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/2] base: Add support to direct OpenSAF logging to local 
node file [#2306]

Hi Hans,

Please check my response with [Minh]

Thanks

Minh


On 18/04/18 22:40, Hans Nordeback wrote:

Hi Minh,

ack, code review only. Some comments below.

/Thanks HansN


On 04/12/2018 01:12 AM, Minh Chau wrote:

Unify TraceLog and MdsLog class to one class (TraceLog) so it can be
used as a common log client.
Add new instance TraceLog for OpenSAF logging to local file, which
can be enabled/disabled via environment variable OSAF_LOCAL_NODE_LOG
---
   src/base/logtrace.cc | 167
++-
   src/base/logtrace.h  |  50 +--
   src/mds/mds_log.cc   | 114 +++
   3 files changed, 140 insertions(+), 191 deletions(-)

diff --git a/src/base/logtrace.cc b/src/base/logtrace.cc index
b046fab..857e31c 100644
--- a/src/base/logtrace.cc
+++ b/src/base/logtrace.cc
@@ -36,15 +36,10 @@
   #include 
   #include 
   #include 
-#include "base/buffer.h"
-#include "base/conf.h"
-#include "base/log_message.h"
-#include "base/macros.h"
-#include "base/mutex.h"
+#include "base/getenv.h"
   #include "base/ncsgl_defs.h"
   #include "base/osaf_utility.h"
   #include "base/time.h"
-#include "base/unix_client_socket.h"
   #include "dtm/common/osaflog_protocol.h"
     namespace global {
@@ -55,65 +50,38 @@ const char *const prefix_name[] = {"EM", "AL",
"CR", "ER", "WA", "NO", "IN",
  "T6", "T7", "T8", ">>", "<<"};
   char *msg_id;
   int logmask;
+const char* osaf_log_file = "osaf.log"; bool enable_osaf_log =
+false;
     }  // namespace global
   -class TraceLog {
- public:
-  static bool Init();
-  static void Log(base::LogMessage::Severity severity, const char
*fmt,
-  va_list ap);
-
- private:
-  TraceLog(const std::string , const std::string _name,
-   uint32_t proc_id, const std::string _id,
-   const std::string _name);
-  void LogInternal(base::LogMessage::Severity severity, const char
*fmt,
-   va_list ap);
-  static constexpr const uint32_t kMaxSequenceId =
uint32_t{0x7fff};
-  static TraceLog *instance_;
-  const base::LogMessage::HostName fqdn_;
-  const base::LogMessage::AppName app_name_;
-  const base::LogMessage::ProcId proc_id_;
-  const base::LogMessage::MsgId msg_id_;
-  uint32_t sequence_id_;
-  base::UnixClientSocket log_socket_;
-  base::Buffer<512> buffer_;
-  base::Mutex mutex_;
-
-  DELETE_COPY_AND_MOVE_OPERATORS(TraceLog);
-};
-
-TraceLog *TraceLog::instance_ = nullptr;
-
-TraceLog::TraceLog(const std::string , const std::string
_name,
-   uint32_t proc_id, const std::string _id,
-   const std::string _name)
-    : fqdn_{base::LogMessage::HostName{fqdn}},
-  app_name_{base::LogMessage::AppName{app_name}},
- proc_id_{base::LogMessage::ProcId{std::to_string(proc_id)}},
-  msg_id_{base::LogMessage::MsgId{msg_id}},
-  sequence_id_{1},
-  log_socket_{socket_name, base::UnixSocket::kBlocking},
-  buffer_{},
-  mutex_{} {}
-
-bool TraceLog::Init() {
-  if (instance_ != nullptr) return false;
-  char app_name[49];
-  char pid_path[1024];

[HansN] instead of static global use unnamed namespaces instead. Also
try to avoid globals, why change Log and Init from static members? (An
alternative is to use a singleton instead, if needed)

[Minh] Changing Log() and Init() not to be static because I am not using 
singleton any more. Before this ticket, we have 2 singleton, one for TraceLog 
and one for MdsLog, they are mostly looking similar, so don't want to make a 
third singleton for new osaf log.
[HansN]  TraceLog and MdsLog::Init and Log where static member functions 
belonging to respective class namespace, now static globals are used.  Not that 
big deal but good to avoid global data.

+static TraceLog gl_trace;
+static TraceLog gl_osaflog;
+TraceLog::TraceLog()
+    : sequence_id_{1}, buffer_{} {
+  mutex_ = nullptr;
+  log_socket_ = nullptr;
+}
+
+TraceLog::~TraceLog() {
+  if (mutex_) delete mutex_;
+  if (log_socket_) delete log_socket_; } bool TraceLog::Init(const
+char *msg_id, WriteMode mode) {
+  char app_name[NAME_MAX];
+  char pid_path[PATH_MAX];
     uint32_t process_id = getpid();
     char *token, *saveptr;
     char *pid_name = nullptr;
   -  memset(app_name, 0, 49);
-  memset(pid_path, 0, 1024);
+  memset(app_name, 0, NAME_MAX);
+  memset(pid_path, 0, PATH_MAX);
       snprintf(pid_path, sizeof(pid_path), "/proc/%" PRIu32
"/cmdline", process_id);
     

Re: [devel] [PATCH 1/2] base: Add support to direct OpenSAF logging to local node file [#2306]

2018-04-18 Thread Hans Nordebäck
Hi Minh,

 See my comments below.

/Thanks HansN

-Original Message-
From: Minh Hon Chau [mailto:minh.c...@dektech.com.au] 
Sent: den 18 april 2018 15:20
To: Hans Nordebäck ; Anders Widell 
; ravisekhar.ko...@oracle.com
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/2] base: Add support to direct OpenSAF logging to local 
node file [#2306]

Hi Hans,

Please check my response with [Minh]

Thanks

Minh


On 18/04/18 22:40, Hans Nordeback wrote:
> Hi Minh,
>
> ack, code review only. Some comments below.
>
> /Thanks HansN
>
>
> On 04/12/2018 01:12 AM, Minh Chau wrote:
>> Unify TraceLog and MdsLog class to one class (TraceLog) so it can be 
>> used as a common log client.
>> Add new instance TraceLog for OpenSAF logging to local file, which 
>> can be enabled/disabled via environment variable OSAF_LOCAL_NODE_LOG
>> ---
>>   src/base/logtrace.cc | 167
>> ++-
>>   src/base/logtrace.h  |  50 +--
>>   src/mds/mds_log.cc   | 114 +++
>>   3 files changed, 140 insertions(+), 191 deletions(-)
>>
>> diff --git a/src/base/logtrace.cc b/src/base/logtrace.cc index 
>> b046fab..857e31c 100644
>> --- a/src/base/logtrace.cc
>> +++ b/src/base/logtrace.cc
>> @@ -36,15 +36,10 @@
>>   #include 
>>   #include 
>>   #include 
>> -#include "base/buffer.h"
>> -#include "base/conf.h"
>> -#include "base/log_message.h"
>> -#include "base/macros.h"
>> -#include "base/mutex.h"
>> +#include "base/getenv.h"
>>   #include "base/ncsgl_defs.h"
>>   #include "base/osaf_utility.h"
>>   #include "base/time.h"
>> -#include "base/unix_client_socket.h"
>>   #include "dtm/common/osaflog_protocol.h"
>>     namespace global {
>> @@ -55,65 +50,38 @@ const char *const prefix_name[] = {"EM", "AL", 
>> "CR", "ER", "WA", "NO", "IN",
>>  "T6", "T7", "T8", ">>", "<<"};
>>   char *msg_id;
>>   int logmask;
>> +const char* osaf_log_file = "osaf.log"; bool enable_osaf_log = 
>> +false;
>>     }  // namespace global
>>   -class TraceLog {
>> - public:
>> -  static bool Init();
>> -  static void Log(base::LogMessage::Severity severity, const char 
>> *fmt,
>> -  va_list ap);
>> -
>> - private:
>> -  TraceLog(const std::string , const std::string _name,
>> -   uint32_t proc_id, const std::string _id,
>> -   const std::string _name);
>> -  void LogInternal(base::LogMessage::Severity severity, const char 
>> *fmt,
>> -   va_list ap);
>> -  static constexpr const uint32_t kMaxSequenceId = 
>> uint32_t{0x7fff};
>> -  static TraceLog *instance_;
>> -  const base::LogMessage::HostName fqdn_;
>> -  const base::LogMessage::AppName app_name_;
>> -  const base::LogMessage::ProcId proc_id_;
>> -  const base::LogMessage::MsgId msg_id_;
>> -  uint32_t sequence_id_;
>> -  base::UnixClientSocket log_socket_;
>> -  base::Buffer<512> buffer_;
>> -  base::Mutex mutex_;
>> -
>> -  DELETE_COPY_AND_MOVE_OPERATORS(TraceLog);
>> -};
>> -
>> -TraceLog *TraceLog::instance_ = nullptr;
>> -
>> -TraceLog::TraceLog(const std::string , const std::string 
>> _name,
>> -   uint32_t proc_id, const std::string _id,
>> -   const std::string _name)
>> -    : fqdn_{base::LogMessage::HostName{fqdn}},
>> -  app_name_{base::LogMessage::AppName{app_name}},
>> - proc_id_{base::LogMessage::ProcId{std::to_string(proc_id)}},
>> -  msg_id_{base::LogMessage::MsgId{msg_id}},
>> -  sequence_id_{1},
>> -  log_socket_{socket_name, base::UnixSocket::kBlocking},
>> -  buffer_{},
>> -  mutex_{} {}
>> -
>> -bool TraceLog::Init() {
>> -  if (instance_ != nullptr) return false;
>> -  char app_name[49];
>> -  char pid_path[1024];
> [HansN] instead of static global use unnamed namespaces instead. Also 
> try to avoid globals, why change Log and Init from static members? (An 
> alternative is to use a singleton instead, if needed)
[Minh] Changing Log() and Init() not to be static because I am not using 
singleton any more. Before this ticket, we have 2 singleton, one for TraceLog 
and one for MdsLog, they are mostly looking similar, so don't want to make a 
third singleton for new osaf log.
[HansN]  TraceLog and MdsLog::Init and Log where static member functions 
belonging to respective class namespace, now static globals are used.  Not that 
big deal but good to avoid global data. 
>> +static TraceLog gl_trace;
>> +static TraceLog gl_osaflog;
>> +TraceLog::TraceLog()
>> +    : sequence_id_{1}, buffer_{} {
>> +  mutex_ = nullptr;
>> +  log_socket_ = nullptr;
>> +}
>> +
>> +TraceLog::~TraceLog() {
>> +  if (mutex_) delete mutex_;
>> +  if (log_socket_) delete log_socket_; } bool TraceLog::Init(const 
>> +char *msg_id, WriteMode mode) {
>> +  char app_name[NAME_MAX];
>> +  char pid_path[PATH_MAX];
>>     uint32_t process_id = getpid();
>>     char *token, *saveptr;
>>     char *pid_name = nullptr;
>>   -  

Re: [devel] [PATCH 1/2] base: Add support to direct OpenSAF logging to local node file [#2306]

2018-04-18 Thread Minh Hon Chau

Hi Hans,

Please check my response with [Minh]

Thanks

Minh


On 18/04/18 22:40, Hans Nordeback wrote:

Hi Minh,

ack, code review only. Some comments below.

/Thanks HansN


On 04/12/2018 01:12 AM, Minh Chau wrote:

Unify TraceLog and MdsLog class to one class (TraceLog) so it can be
used as a common log client.
Add new instance TraceLog for OpenSAF logging to local file, which can
be enabled/disabled via environment variable OSAF_LOCAL_NODE_LOG
---
  src/base/logtrace.cc | 167 
++-

  src/base/logtrace.h  |  50 +--
  src/mds/mds_log.cc   | 114 +++
  3 files changed, 140 insertions(+), 191 deletions(-)

diff --git a/src/base/logtrace.cc b/src/base/logtrace.cc
index b046fab..857e31c 100644
--- a/src/base/logtrace.cc
+++ b/src/base/logtrace.cc
@@ -36,15 +36,10 @@
  #include 
  #include 
  #include 
-#include "base/buffer.h"
-#include "base/conf.h"
-#include "base/log_message.h"
-#include "base/macros.h"
-#include "base/mutex.h"
+#include "base/getenv.h"
  #include "base/ncsgl_defs.h"
  #include "base/osaf_utility.h"
  #include "base/time.h"
-#include "base/unix_client_socket.h"
  #include "dtm/common/osaflog_protocol.h"
    namespace global {
@@ -55,65 +50,38 @@ const char *const prefix_name[] = {"EM", "AL", 
"CR", "ER", "WA", "NO", "IN",

 "T6", "T7", "T8", ">>", "<<"};
  char *msg_id;
  int logmask;
+const char* osaf_log_file = "osaf.log";
+bool enable_osaf_log = false;
    }  // namespace global
  -class TraceLog {
- public:
-  static bool Init();
-  static void Log(base::LogMessage::Severity severity, const char *fmt,
-  va_list ap);
-
- private:
-  TraceLog(const std::string , const std::string _name,
-   uint32_t proc_id, const std::string _id,
-   const std::string _name);
-  void LogInternal(base::LogMessage::Severity severity, const char 
*fmt,

-   va_list ap);
-  static constexpr const uint32_t kMaxSequenceId = 
uint32_t{0x7fff};

-  static TraceLog *instance_;
-  const base::LogMessage::HostName fqdn_;
-  const base::LogMessage::AppName app_name_;
-  const base::LogMessage::ProcId proc_id_;
-  const base::LogMessage::MsgId msg_id_;
-  uint32_t sequence_id_;
-  base::UnixClientSocket log_socket_;
-  base::Buffer<512> buffer_;
-  base::Mutex mutex_;
-
-  DELETE_COPY_AND_MOVE_OPERATORS(TraceLog);
-};
-
-TraceLog *TraceLog::instance_ = nullptr;
-
-TraceLog::TraceLog(const std::string , const std::string 
_name,

-   uint32_t proc_id, const std::string _id,
-   const std::string _name)
-    : fqdn_{base::LogMessage::HostName{fqdn}},
-  app_name_{base::LogMessage::AppName{app_name}},
- proc_id_{base::LogMessage::ProcId{std::to_string(proc_id)}},
-  msg_id_{base::LogMessage::MsgId{msg_id}},
-  sequence_id_{1},
-  log_socket_{socket_name, base::UnixSocket::kBlocking},
-  buffer_{},
-  mutex_{} {}
-
-bool TraceLog::Init() {
-  if (instance_ != nullptr) return false;
-  char app_name[49];
-  char pid_path[1024];
[HansN] instead of static global use unnamed namespaces instead. Also 
try to avoid globals, why change Log and Init from

static members? (An alternative is to use a singleton instead, if needed)
[Minh] Changing Log() and Init() not to be static because I am not using 
singleton any more. Before this ticket, we have 2 singleton, one for 
TraceLog and one for MdsLog, they are mostly looking similar, so don't 
want to make a third singleton for new osaf log.

+static TraceLog gl_trace;
+static TraceLog gl_osaflog;
+TraceLog::TraceLog()
+    : sequence_id_{1}, buffer_{} {
+  mutex_ = nullptr;
+  log_socket_ = nullptr;
+}
+
+TraceLog::~TraceLog() {
+  if (mutex_) delete mutex_;
+  if (log_socket_) delete log_socket_;
+}
+bool TraceLog::Init(const char *msg_id, WriteMode mode) {
+  char app_name[NAME_MAX];
+  char pid_path[PATH_MAX];
    uint32_t process_id = getpid();
    char *token, *saveptr;
    char *pid_name = nullptr;
  -  memset(app_name, 0, 49);
-  memset(pid_path, 0, 1024);
+  memset(app_name, 0, NAME_MAX);
+  memset(pid_path, 0, PATH_MAX);
      snprintf(pid_path, sizeof(pid_path), "/proc/%" PRIu32 
"/cmdline", process_id);

    FILE *f = fopen(pid_path, "r");
    if (f != nullptr) {
  size_t size;
-    size = fread(pid_path, sizeof(char), 1024, f);
+    size = fread(pid_path, sizeof(char), PATH_MAX, f);
  if (size > 0) {
    if ('\n' == pid_path[size - 1]) pid_path[size - 1] = '\0';
  }
@@ -131,19 +99,28 @@ bool TraceLog::Init() {
    }
    base::Conf::InitFullyQualifiedDomainName();
    const std::string  = base::Conf::FullyQualifiedDomainName();
-  instance_ = new TraceLog{fqdn, app_name, process_id, global::msg_id,
-   Osaflog::kServerSocketPath};
-  return instance_ != nullptr;
+
+  fqdn_ = base::LogMessage::HostName(fqdn);
+  app_name_ = base::LogMessage::AppName(app_name);
+  proc_id_ = 

Re: [devel] [PATCH 1/2] base: Add support to direct OpenSAF logging to local node file [#2306]

2018-04-18 Thread Hans Nordeback

Hi Minh,

ack, code review only. Some comments below.

/Thanks HansN


On 04/12/2018 01:12 AM, Minh Chau wrote:

Unify TraceLog and MdsLog class to one class (TraceLog) so it can be
used as a common log client.
Add new instance TraceLog for OpenSAF logging to local file, which can
be enabled/disabled via environment variable OSAF_LOCAL_NODE_LOG
---
  src/base/logtrace.cc | 167 ++-
  src/base/logtrace.h  |  50 +--
  src/mds/mds_log.cc   | 114 +++
  3 files changed, 140 insertions(+), 191 deletions(-)

diff --git a/src/base/logtrace.cc b/src/base/logtrace.cc
index b046fab..857e31c 100644
--- a/src/base/logtrace.cc
+++ b/src/base/logtrace.cc
@@ -36,15 +36,10 @@
  #include 
  #include 
  #include 
-#include "base/buffer.h"
-#include "base/conf.h"
-#include "base/log_message.h"
-#include "base/macros.h"
-#include "base/mutex.h"
+#include "base/getenv.h"
  #include "base/ncsgl_defs.h"
  #include "base/osaf_utility.h"
  #include "base/time.h"
-#include "base/unix_client_socket.h"
  #include "dtm/common/osaflog_protocol.h"
  
  namespace global {

@@ -55,65 +50,38 @@ const char *const prefix_name[] = {"EM", "AL", "CR", "ER", "WA", 
"NO", "IN",
 "T6", "T7", "T8", ">>", "<<"};
  char *msg_id;
  int logmask;
+const char* osaf_log_file = "osaf.log";
+bool enable_osaf_log = false;
  
  }  // namespace global
  
-class TraceLog {

- public:
-  static bool Init();
-  static void Log(base::LogMessage::Severity severity, const char *fmt,
-  va_list ap);
-
- private:
-  TraceLog(const std::string , const std::string _name,
-   uint32_t proc_id, const std::string _id,
-   const std::string _name);
-  void LogInternal(base::LogMessage::Severity severity, const char *fmt,
-   va_list ap);
-  static constexpr const uint32_t kMaxSequenceId = uint32_t{0x7fff};
-  static TraceLog *instance_;
-  const base::LogMessage::HostName fqdn_;
-  const base::LogMessage::AppName app_name_;
-  const base::LogMessage::ProcId proc_id_;
-  const base::LogMessage::MsgId msg_id_;
-  uint32_t sequence_id_;
-  base::UnixClientSocket log_socket_;
-  base::Buffer<512> buffer_;
-  base::Mutex mutex_;
-
-  DELETE_COPY_AND_MOVE_OPERATORS(TraceLog);
-};
-
-TraceLog *TraceLog::instance_ = nullptr;
-
-TraceLog::TraceLog(const std::string , const std::string _name,
-   uint32_t proc_id, const std::string _id,
-   const std::string _name)
-: fqdn_{base::LogMessage::HostName{fqdn}},
-  app_name_{base::LogMessage::AppName{app_name}},
-  proc_id_{base::LogMessage::ProcId{std::to_string(proc_id)}},
-  msg_id_{base::LogMessage::MsgId{msg_id}},
-  sequence_id_{1},
-  log_socket_{socket_name, base::UnixSocket::kBlocking},
-  buffer_{},
-  mutex_{} {}
-
-bool TraceLog::Init() {
-  if (instance_ != nullptr) return false;
-  char app_name[49];
-  char pid_path[1024];
[HansN] instead of static global use unnamed namespaces instead. Also 
try to avoid globals, why change Log and Init from

static members? (An alternative is to use a singleton instead, if needed)

+static TraceLog gl_trace;
+static TraceLog gl_osaflog;
+TraceLog::TraceLog()
+: sequence_id_{1}, buffer_{} {
+  mutex_ = nullptr;
+  log_socket_ = nullptr;
+}
+
+TraceLog::~TraceLog() {
+  if (mutex_) delete mutex_;
+  if (log_socket_) delete log_socket_;
+}
+bool TraceLog::Init(const char *msg_id, WriteMode mode) {
+  char app_name[NAME_MAX];
+  char pid_path[PATH_MAX];
uint32_t process_id = getpid();
char *token, *saveptr;
char *pid_name = nullptr;
  
-  memset(app_name, 0, 49);

-  memset(pid_path, 0, 1024);
+  memset(app_name, 0, NAME_MAX);
+  memset(pid_path, 0, PATH_MAX);
  
snprintf(pid_path, sizeof(pid_path), "/proc/%" PRIu32 "/cmdline", process_id);

FILE *f = fopen(pid_path, "r");
if (f != nullptr) {
  size_t size;
-size = fread(pid_path, sizeof(char), 1024, f);
+size = fread(pid_path, sizeof(char), PATH_MAX, f);
  if (size > 0) {
if ('\n' == pid_path[size - 1]) pid_path[size - 1] = '\0';
  }
@@ -131,19 +99,28 @@ bool TraceLog::Init() {
}
base::Conf::InitFullyQualifiedDomainName();
const std::string  = base::Conf::FullyQualifiedDomainName();
-  instance_ = new TraceLog{fqdn, app_name, process_id, global::msg_id,
-   Osaflog::kServerSocketPath};
-  return instance_ != nullptr;
+
+  fqdn_ = base::LogMessage::HostName(fqdn);
+  app_name_ = base::LogMessage::AppName(app_name);
+  proc_id_ = base::LogMessage::ProcId{std::to_string(process_id)};
+  msg_id_ = base::LogMessage::MsgId{msg_id};
+  log_socket_ = new base::UnixClientSocket{Osaflog::kServerSocketPath,
+static_cast(mode)};
+  mutex_ = new base::Mutex{};
+
+  return true;
  }
  
  void TraceLog::Log(base::LogMessage::Severity severity, const char *fmt,

 va_list ap) {
-  if (instance_ != 

Re: [devel] [PATCH 1/2] base: Add support to direct OpenSAF logging to local node file [#2306]

2018-04-17 Thread Minh Hon Chau

Hi,

If you have time, can you please review the patch?

I have done self review, a few things I think it needs to update

- The redundant headers file in mds_log.cc that were needed for MdsLog 
class should be removed


- The TraceLog::Init needs to return if the Init() has been already done.

- With the support of #2838, the log/trace can be written if application 
is running as non-root user, the new OSAF_LOCAL_NODE_LOG env also needs 
to be documented in a PR doc.


- Another thing, if application uses 2 or more services (say NTF, AMF) 
and enables both NTFSV_TRACE_PATHNAME and AVA_TRACE_PATHNAME as in two 
different paths, but only the first variable path is used for both 
agents' tracing. This seems to be there before #2306, changing this may 
cause a NBC, so maybe just document it somehow


Regards,
Minh
On 12/04/18 09:12, Minh Chau wrote:

Unify TraceLog and MdsLog class to one class (TraceLog) so it can be
used as a common log client.
Add new instance TraceLog for OpenSAF logging to local file, which can
be enabled/disabled via environment variable OSAF_LOCAL_NODE_LOG
---
  src/base/logtrace.cc | 167 ++-
  src/base/logtrace.h  |  50 +--
  src/mds/mds_log.cc   | 114 +++
  3 files changed, 140 insertions(+), 191 deletions(-)

diff --git a/src/base/logtrace.cc b/src/base/logtrace.cc
index b046fab..857e31c 100644
--- a/src/base/logtrace.cc
+++ b/src/base/logtrace.cc
@@ -36,15 +36,10 @@
  #include 
  #include 
  #include 
-#include "base/buffer.h"
-#include "base/conf.h"
-#include "base/log_message.h"
-#include "base/macros.h"
-#include "base/mutex.h"
+#include "base/getenv.h"
  #include "base/ncsgl_defs.h"
  #include "base/osaf_utility.h"
  #include "base/time.h"
-#include "base/unix_client_socket.h"
  #include "dtm/common/osaflog_protocol.h"
  
  namespace global {

@@ -55,65 +50,38 @@ const char *const prefix_name[] = {"EM", "AL", "CR", "ER", "WA", 
"NO", "IN",
 "T6", "T7", "T8", ">>", "<<"};
  char *msg_id;
  int logmask;
+const char* osaf_log_file = "osaf.log";
+bool enable_osaf_log = false;
  
  }  // namespace global
  
-class TraceLog {

- public:
-  static bool Init();
-  static void Log(base::LogMessage::Severity severity, const char *fmt,
-  va_list ap);
-
- private:
-  TraceLog(const std::string , const std::string _name,
-   uint32_t proc_id, const std::string _id,
-   const std::string _name);
-  void LogInternal(base::LogMessage::Severity severity, const char *fmt,
-   va_list ap);
-  static constexpr const uint32_t kMaxSequenceId = uint32_t{0x7fff};
-  static TraceLog *instance_;
-  const base::LogMessage::HostName fqdn_;
-  const base::LogMessage::AppName app_name_;
-  const base::LogMessage::ProcId proc_id_;
-  const base::LogMessage::MsgId msg_id_;
-  uint32_t sequence_id_;
-  base::UnixClientSocket log_socket_;
-  base::Buffer<512> buffer_;
-  base::Mutex mutex_;
-
-  DELETE_COPY_AND_MOVE_OPERATORS(TraceLog);
-};
-
-TraceLog *TraceLog::instance_ = nullptr;
-
-TraceLog::TraceLog(const std::string , const std::string _name,
-   uint32_t proc_id, const std::string _id,
-   const std::string _name)
-: fqdn_{base::LogMessage::HostName{fqdn}},
-  app_name_{base::LogMessage::AppName{app_name}},
-  proc_id_{base::LogMessage::ProcId{std::to_string(proc_id)}},
-  msg_id_{base::LogMessage::MsgId{msg_id}},
-  sequence_id_{1},
-  log_socket_{socket_name, base::UnixSocket::kBlocking},
-  buffer_{},
-  mutex_{} {}
-
-bool TraceLog::Init() {
-  if (instance_ != nullptr) return false;
-  char app_name[49];
-  char pid_path[1024];
+static TraceLog gl_trace;
+static TraceLog gl_osaflog;
+TraceLog::TraceLog()
+: sequence_id_{1}, buffer_{} {
+  mutex_ = nullptr;
+  log_socket_ = nullptr;
+}
+
+TraceLog::~TraceLog() {
+  if (mutex_) delete mutex_;
+  if (log_socket_) delete log_socket_;
+}
+bool TraceLog::Init(const char *msg_id, WriteMode mode) {
+  char app_name[NAME_MAX];
+  char pid_path[PATH_MAX];
uint32_t process_id = getpid();
char *token, *saveptr;
char *pid_name = nullptr;
  
-  memset(app_name, 0, 49);

-  memset(pid_path, 0, 1024);
+  memset(app_name, 0, NAME_MAX);
+  memset(pid_path, 0, PATH_MAX);
  
snprintf(pid_path, sizeof(pid_path), "/proc/%" PRIu32 "/cmdline", process_id);

FILE *f = fopen(pid_path, "r");
if (f != nullptr) {
  size_t size;
-size = fread(pid_path, sizeof(char), 1024, f);
+size = fread(pid_path, sizeof(char), PATH_MAX, f);
  if (size > 0) {
if ('\n' == pid_path[size - 1]) pid_path[size - 1] = '\0';
  }
@@ -131,19 +99,28 @@ bool TraceLog::Init() {
}
base::Conf::InitFullyQualifiedDomainName();
const std::string  = base::Conf::FullyQualifiedDomainName();
-  instance_ = new TraceLog{fqdn, app_name, process_id, global::msg_id,
-   

[devel] [PATCH 1/2] base: Add support to direct OpenSAF logging to local node file [#2306]

2018-04-11 Thread Minh Chau
Unify TraceLog and MdsLog class to one class (TraceLog) so it can be
used as a common log client.
Add new instance TraceLog for OpenSAF logging to local file, which can
be enabled/disabled via environment variable OSAF_LOCAL_NODE_LOG
---
 src/base/logtrace.cc | 167 ++-
 src/base/logtrace.h  |  50 +--
 src/mds/mds_log.cc   | 114 +++
 3 files changed, 140 insertions(+), 191 deletions(-)

diff --git a/src/base/logtrace.cc b/src/base/logtrace.cc
index b046fab..857e31c 100644
--- a/src/base/logtrace.cc
+++ b/src/base/logtrace.cc
@@ -36,15 +36,10 @@
 #include 
 #include 
 #include 
-#include "base/buffer.h"
-#include "base/conf.h"
-#include "base/log_message.h"
-#include "base/macros.h"
-#include "base/mutex.h"
+#include "base/getenv.h"
 #include "base/ncsgl_defs.h"
 #include "base/osaf_utility.h"
 #include "base/time.h"
-#include "base/unix_client_socket.h"
 #include "dtm/common/osaflog_protocol.h"
 
 namespace global {
@@ -55,65 +50,38 @@ const char *const prefix_name[] = {"EM", "AL", "CR", "ER", 
"WA", "NO", "IN",
"T6", "T7", "T8", ">>", "<<"};
 char *msg_id;
 int logmask;
+const char* osaf_log_file = "osaf.log";
+bool enable_osaf_log = false;
 
 }  // namespace global
 
-class TraceLog {
- public:
-  static bool Init();
-  static void Log(base::LogMessage::Severity severity, const char *fmt,
-  va_list ap);
-
- private:
-  TraceLog(const std::string , const std::string _name,
-   uint32_t proc_id, const std::string _id,
-   const std::string _name);
-  void LogInternal(base::LogMessage::Severity severity, const char *fmt,
-   va_list ap);
-  static constexpr const uint32_t kMaxSequenceId = uint32_t{0x7fff};
-  static TraceLog *instance_;
-  const base::LogMessage::HostName fqdn_;
-  const base::LogMessage::AppName app_name_;
-  const base::LogMessage::ProcId proc_id_;
-  const base::LogMessage::MsgId msg_id_;
-  uint32_t sequence_id_;
-  base::UnixClientSocket log_socket_;
-  base::Buffer<512> buffer_;
-  base::Mutex mutex_;
-
-  DELETE_COPY_AND_MOVE_OPERATORS(TraceLog);
-};
-
-TraceLog *TraceLog::instance_ = nullptr;
-
-TraceLog::TraceLog(const std::string , const std::string _name,
-   uint32_t proc_id, const std::string _id,
-   const std::string _name)
-: fqdn_{base::LogMessage::HostName{fqdn}},
-  app_name_{base::LogMessage::AppName{app_name}},
-  proc_id_{base::LogMessage::ProcId{std::to_string(proc_id)}},
-  msg_id_{base::LogMessage::MsgId{msg_id}},
-  sequence_id_{1},
-  log_socket_{socket_name, base::UnixSocket::kBlocking},
-  buffer_{},
-  mutex_{} {}
-
-bool TraceLog::Init() {
-  if (instance_ != nullptr) return false;
-  char app_name[49];
-  char pid_path[1024];
+static TraceLog gl_trace;
+static TraceLog gl_osaflog;
+TraceLog::TraceLog()
+: sequence_id_{1}, buffer_{} {
+  mutex_ = nullptr;
+  log_socket_ = nullptr;
+}
+
+TraceLog::~TraceLog() {
+  if (mutex_) delete mutex_;
+  if (log_socket_) delete log_socket_;
+}
+bool TraceLog::Init(const char *msg_id, WriteMode mode) {
+  char app_name[NAME_MAX];
+  char pid_path[PATH_MAX];
   uint32_t process_id = getpid();
   char *token, *saveptr;
   char *pid_name = nullptr;
 
-  memset(app_name, 0, 49);
-  memset(pid_path, 0, 1024);
+  memset(app_name, 0, NAME_MAX);
+  memset(pid_path, 0, PATH_MAX);
 
   snprintf(pid_path, sizeof(pid_path), "/proc/%" PRIu32 "/cmdline", 
process_id);
   FILE *f = fopen(pid_path, "r");
   if (f != nullptr) {
 size_t size;
-size = fread(pid_path, sizeof(char), 1024, f);
+size = fread(pid_path, sizeof(char), PATH_MAX, f);
 if (size > 0) {
   if ('\n' == pid_path[size - 1]) pid_path[size - 1] = '\0';
 }
@@ -131,19 +99,28 @@ bool TraceLog::Init() {
   }
   base::Conf::InitFullyQualifiedDomainName();
   const std::string  = base::Conf::FullyQualifiedDomainName();
-  instance_ = new TraceLog{fqdn, app_name, process_id, global::msg_id,
-   Osaflog::kServerSocketPath};
-  return instance_ != nullptr;
+
+  fqdn_ = base::LogMessage::HostName(fqdn);
+  app_name_ = base::LogMessage::AppName(app_name);
+  proc_id_ = base::LogMessage::ProcId{std::to_string(process_id)};
+  msg_id_ = base::LogMessage::MsgId{msg_id};
+  log_socket_ = new base::UnixClientSocket{Osaflog::kServerSocketPath,
+static_cast(mode)};
+  mutex_ = new base::Mutex{};
+
+  return true;
 }
 
 void TraceLog::Log(base::LogMessage::Severity severity, const char *fmt,
va_list ap) {
-  if (instance_ != nullptr) instance_->LogInternal(severity, fmt, ap);
+  if (log_socket_ != nullptr && mutex_ != nullptr) {
+LogInternal(severity, fmt, ap);
+  }
 }
 
 void TraceLog::LogInternal(base::LogMessage::Severity severity, const char 
*fmt,
va_list ap) {
-  base::Lock lock(mutex_);
+  base::Lock lock(*mutex_);
   uint32_t id =