subject:"Re\\\\\\\: \\\\\\\[devel\\\\\\\] \\\\\\\[PATCH 1\\\\\\\/1\\\\\\\] log\\\\\\\: fix cppcheck, cpplint and reorganize headers \\\\\\\- part 1 \\\\\\\[#2445\\\\\\\]"

Re: [devel] [PATCH 1/1] log: fix cppcheck, cpplint and reorganize headers - part 1 [#2445]

2017-08-09 Thread Vu Minh Nguyen

Hi Canh,

Ack with comments, with [Vu].

Regards, Vu

> -Original Message-
> From: Canh Van Truong [mailto:canh.v.tru...@dektech.com.au]
> Sent: Thursday, July 20, 2017 1:31 PM
> To: lennart.l...@ericsson.com; vu.m.ngu...@dektech.com.au;
> mahesh.va...@oracle.com
> Cc: opensaf-devel@lists.sourceforge.net; Canh Van Truong
> 
> Subject: [PATCH 1/1] log: fix cppcheck, cpplint and reorganize headers -
part
> 1 [#2445]
> 
>Fix cppcheck, cpplint, replace nullptr for following files:
> - lgs_amf.*, lgs_config.*, lgs_dest.*, lgs_evt.*, lgs_file.*,
>   lgs_filehdl.*, lgs_nildest.*, lgs_unixsock_dest.*
>   lgs_util.*, lgs_dest_test.*
> ---
>  src/log/logd/lgs_amf.cc  |  25 ++--
>  src/log/logd/lgs_config.cc   | 238
+++
>  src/log/logd/lgs_config.h|   3 +-
>  src/log/logd/lgs_dest.cc |   2 +-
>  src/log/logd/lgs_dest.h  |   6 +-
>  src/log/logd/lgs_evt.cc  | 111 +-
>  src/log/logd/lgs_evt.h   |  17 ++-
>  src/log/logd/lgs_file.cc |  56 +
>  src/log/logd/lgs_filehdl.cc  |  84 +++---
>  src/log/logd/lgs_nildest.h   |   6 +-
>  src/log/logd/lgs_recov.cc|   4 +-
>  src/log/logd/lgs_stream.cc   |  16 +--
>  src/log/logd/lgs_unixsock_dest.h |   6 +-
>  src/log/logd/lgs_util.cc |  58 +-
>  src/log/logd/lgs_util.h  |   7 +-
>  src/log/tests/lgs_dest_test.cc   |   2 +-
>  16 files changed, 293 insertions(+), 348 deletions(-)
> 
> diff --git a/src/log/logd/lgs_amf.cc b/src/log/logd/lgs_amf.cc
> index 434d40861..6821cb859 100644
> --- a/src/log/logd/lgs_amf.cc
> +++ b/src/log/logd/lgs_amf.cc
> @@ -20,9 +20,9 @@
>   */
> 
>  #include "nid/agent/nid_start_util.h"
> -#include "log/logd/lgs.h"
> -#include "lgs_config.h"
>  #include "osaf/immutil/immutil.h"
> +#include "log/logd/lgs.h"
> +#include "log/logd/lgs_config.h"
> 
>  static void close_all_files() {
>log_stream_t *stream;
> @@ -124,13 +124,11 @@ static SaAisErrorT
> amf_standby_state_handler(lgs_cb_t *cb,
> 
> **
> ***/
>  static SaAisErrorT amf_quiescing_state_handler(lgs_cb_t *cb,
> SaInvocationT invocation)
{
> -  SaAisErrorT ais_rc = SA_AIS_OK;
> -
>TRACE_ENTER2("HA QUIESCING request");
>close_all_files();
> 
>/* Give up our IMM OI implementer role */
> -  ais_rc = immutil_saImmOiImplementerClear(cb->immOiHandle);
> +  SaAisErrorT ais_rc = immutil_saImmOiImplementerClear(cb-
> >immOiHandle);
>if (ais_rc != SA_AIS_OK) {
>  LOG_WA("immutil_saImmOiImplementerClear failed: %s",
> saf_error(ais_rc));
>}
> @@ -156,13 +154,11 @@ static SaAisErrorT
> amf_quiescing_state_handler(lgs_cb_t *cb,
>  static SaAisErrorT amf_quiesced_state_handler(lgs_cb_t *cb,
>SaInvocationT invocation) {
>V_DEST_RL mds_role;
> -  SaAisErrorT rc = SA_AIS_OK;
> -
>TRACE_ENTER2("HA AMF QUIESCED STATE request");
>close_all_files();
> 
>/* Give up our IMM OI implementer role */
> -  rc = immutil_saImmOiImplementerClear(cb->immOiHandle);
> +  SaAisErrorT rc = immutil_saImmOiImplementerClear(cb->immOiHandle);
>if (rc != SA_AIS_OK) {
>  LOG_WA("immutil_saImmOiImplementerClear failed: %s", saf_error(rc));
>}
> @@ -410,12 +406,14 @@ static SaAisErrorT amf_healthcheck_start(lgs_cb_t
> *lgs_cb) {
>memset(, 0, sizeof(healthy));
>health_key = getenv("LGSV_ENV_HEALTHCHECK_KEY");
> 
> -  if (health_key == NULL)
> -strcpy((char *)healthy.key, "F1B2");
> +  if (health_key == nullptr)
> +snprintf(reinterpret_cast(healthy.key),
> + SA_AMF_HEALTHCHECK_KEY_MAX, "F1B2");
>else
> -strcpy((char *)healthy.key, health_key);
> +snprintf(reinterpret_cast(healthy.key),
> + SA_AMF_HEALTHCHECK_KEY_MAX, "%s", health_key);
> 
> -  healthy.keyLen = strlen((char *)healthy.key);
> +  healthy.keyLen = strlen(reinterpret_cast(healthy.key));
> 
>error = saAmfHealthcheckStart(lgs_cb->amf_hdl, _cb->comp_name,
> ,
>  SA_AMF_HEALTHCHECK_AMF_INVOKED,
> @@ -485,7 +483,8 @@ SaAisErrorT lgs_amf_init(lgs_cb_t *cb) {
[Vu] `lgs_amf_init()` is public interface of lgs_amf.cc.
Consider to creating its own header file lgs_amf.h and move that function to
that file.
>}
> 
>/* Register component with AMF */
> -  error = saAmfComponentRegister(cb->amf_hdl, >comp_name,
> (SaNameT *)NULL);
> +  error = saAmfComponentRegister(cb->amf_hdl, >comp_name,
> + reinterpret_cast(NULL));
[Vu] 
1) Use nullptr instread.
2) Why need to cast NULL pointer to SaNameT*? 

>if (error != SA_AIS_OK) {
>  LOG_ER("saAmfComponentRegister() FAILED");
>  goto done;
> diff --git a/src/log/logd/lgs_config.cc b/src/log/logd/lgs_config.cc
> index 7b1ad56d6..141aaa2f8 100644
> --- a/src/log/logd/lgs_config.cc

Re: [devel] [PATCH 1/1] log: fix cppcheck, cpplint and reorganize headers - part 1 [#2445]

2017-08-08 Thread A V Mahesh


Hi Canh Van,

ACK, with Same comment as Lennart:

Change `SaAisErrorT om_rc`  similar  in   `static void 
read_logsv_config_obj_2()`  ,


as you already did in   `static SaAisErrorT 
amf_quiescing_state_handler()` in this patch it self.




SaAisErrorT om_rc = immutil_saImmOmInitialize(, NULL, );


-AVM

On 7/20/2017 12:01 PM, Canh Van Truong wrote:

Fix cppcheck, cpplint, replace nullptr for following files:
 - lgs_amf.*, lgs_config.*, lgs_dest.*, lgs_evt.*, lgs_file.*,
   lgs_filehdl.*, lgs_nildest.*, lgs_unixsock_dest.*
   lgs_util.*, lgs_dest_test.*
---
  src/log/logd/lgs_amf.cc  |  25 ++--
  src/log/logd/lgs_config.cc   | 238 +++
  src/log/logd/lgs_config.h|   3 +-
  src/log/logd/lgs_dest.cc |   2 +-
  src/log/logd/lgs_dest.h  |   6 +-
  src/log/logd/lgs_evt.cc  | 111 +-
  src/log/logd/lgs_evt.h   |  17 ++-
  src/log/logd/lgs_file.cc |  56 +
  src/log/logd/lgs_filehdl.cc  |  84 +++---
  src/log/logd/lgs_nildest.h   |   6 +-
  src/log/logd/lgs_recov.cc|   4 +-
  src/log/logd/lgs_stream.cc   |  16 +--
  src/log/logd/lgs_unixsock_dest.h |   6 +-
  src/log/logd/lgs_util.cc |  58 +-
  src/log/logd/lgs_util.h  |   7 +-
  src/log/tests/lgs_dest_test.cc   |   2 +-
  16 files changed, 293 insertions(+), 348 deletions(-)

diff --git a/src/log/logd/lgs_amf.cc b/src/log/logd/lgs_amf.cc
index 434d40861..6821cb859 100644
--- a/src/log/logd/lgs_amf.cc
+++ b/src/log/logd/lgs_amf.cc
@@ -20,9 +20,9 @@
   */
  
  #include "nid/agent/nid_start_util.h"

-#include "log/logd/lgs.h"
-#include "lgs_config.h"
  #include "osaf/immutil/immutil.h"
+#include "log/logd/lgs.h"
+#include "log/logd/lgs_config.h"
  
  static void close_all_files() {

log_stream_t *stream;
@@ -124,13 +124,11 @@ static SaAisErrorT amf_standby_state_handler(lgs_cb_t *cb,
   
*/
  static SaAisErrorT amf_quiescing_state_handler(lgs_cb_t *cb,
 SaInvocationT invocation) {
-  SaAisErrorT ais_rc = SA_AIS_OK;
-
TRACE_ENTER2("HA QUIESCING request");
close_all_files();
  
/* Give up our IMM OI implementer role */

-  ais_rc = immutil_saImmOiImplementerClear(cb->immOiHandle);
+  SaAisErrorT ais_rc = immutil_saImmOiImplementerClear(cb->immOiHandle);
if (ais_rc != SA_AIS_OK) {
  LOG_WA("immutil_saImmOiImplementerClear failed: %s", saf_error(ais_rc));
}
@@ -156,13 +154,11 @@ static SaAisErrorT amf_quiescing_state_handler(lgs_cb_t 
*cb,
  static SaAisErrorT amf_quiesced_state_handler(lgs_cb_t *cb,
SaInvocationT invocation) {
V_DEST_RL mds_role;
-  SaAisErrorT rc = SA_AIS_OK;
-
TRACE_ENTER2("HA AMF QUIESCED STATE request");
close_all_files();
  
/* Give up our IMM OI implementer role */

-  rc = immutil_saImmOiImplementerClear(cb->immOiHandle);
+  SaAisErrorT rc = immutil_saImmOiImplementerClear(cb->immOiHandle);
if (rc != SA_AIS_OK) {
  LOG_WA("immutil_saImmOiImplementerClear failed: %s", saf_error(rc));
}
@@ -410,12 +406,14 @@ static SaAisErrorT amf_healthcheck_start(lgs_cb_t 
*lgs_cb) {
memset(, 0, sizeof(healthy));
health_key = getenv("LGSV_ENV_HEALTHCHECK_KEY");
  
-  if (health_key == NULL)

-strcpy((char *)healthy.key, "F1B2");
+  if (health_key == nullptr)
+snprintf(reinterpret_cast(healthy.key),
+ SA_AMF_HEALTHCHECK_KEY_MAX, "F1B2");
else
-strcpy((char *)healthy.key, health_key);
+snprintf(reinterpret_cast(healthy.key),
+ SA_AMF_HEALTHCHECK_KEY_MAX, "%s", health_key);
  
-  healthy.keyLen = strlen((char *)healthy.key);

+  healthy.keyLen = strlen(reinterpret_cast(healthy.key));
  
error = saAmfHealthcheckStart(lgs_cb->amf_hdl, _cb->comp_name, ,

  SA_AMF_HEALTHCHECK_AMF_INVOKED,
@@ -485,7 +483,8 @@ SaAisErrorT lgs_amf_init(lgs_cb_t *cb) {
}
  
/* Register component with AMF */

-  error = saAmfComponentRegister(cb->amf_hdl, >comp_name, (SaNameT *)NULL);
+  error = saAmfComponentRegister(cb->amf_hdl, >comp_name,
+ reinterpret_cast(NULL));
if (error != SA_AIS_OK) {
  LOG_ER("saAmfComponentRegister() FAILED");
  goto done;
diff --git a/src/log/logd/lgs_config.cc b/src/log/logd/lgs_config.cc
index 7b1ad56d6..141aaa2f8 100644
--- a/src/log/logd/lgs_config.cc
+++ b/src/log/logd/lgs_config.cc
@@ -20,7 +20,7 @@
  #define _GNU_SOURCE
  #endif
  
-#include "lgs_config.h"

+#include "log/logd/lgs_config.h"
  
  #include 

  #include 
@@ -32,11 +32,11 @@
  #include 
  #include 
  
-#include "osaf/configmake.h"

  #include "base/saf_error.h"
  #include "base/osaf_secutil.h"
  #include

Re: [devel] [PATCH 1/1] log: fix cppcheck, cpplint and reorganize headers - part 1 [#2445]

Re: [devel] [PATCH 1/1] log: fix cppcheck, cpplint and reorganize headers - part 1 [#2445]

2 matches

Site Navigation

Mail list logo

Footer information