Re: [devel] [PATCH 1/1] log: fix log supported maximum 2047 characters for long DN [#2525]

[Vu Minh Nguyen]

Ack with one following minor comment.

Consider use C++ google coding rule or Linux Kernel coding rule for naming
these functions: encodeSaNameT()/decodeSaNameT()
e.g: EncodeSaNameT() or encode_sanamet()

And as you do refactor for SaNameT encode/decode, I hope you have run
valgrind/upgrade tests.

Regards, Vu

> -Original Message-
> From: Canh Van Truong [mailto:canh.v.tru...@dektech.com.au]
> Sent: Wednesday, July 12, 2017 3:11 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 log supported maximum 2047 characters for
> long DN [#2525]
> 
> Currently, log support maximum 2047 characters for long DN. it should
> support
> maximum 2048 characters.
> 
> The patch also fixes to check SA_AMF_COMPONENT_NAME maximum 2048
> characters
> and refactor encode and decode for SaNameT to process its name in one
> place
> ---
>  src/log/agent/lga_agent.cc   |   4 +-
>  src/log/agent/lga_mds.cc |  99 ++--
>  src/log/agent/lga_util.cc|   5 +-
>  src/log/apitest/tet_log_longDN.c | 131 +++
> --
>  src/log/logd/lgs_mds.cc  | 137
+++
>  src/log/logd/lgs_recov.cc|   6 +-
>  src/log/logd/lgs_util.cc |   5 +-
>  7 files changed, 210 insertions(+), 177 deletions(-)
> 
> diff --git a/src/log/agent/lga_agent.cc b/src/log/agent/lga_agent.cc
> index e75415763..d5c7c9430 100644
> --- a/src/log/agent/lga_agent.cc
> +++ b/src/log/agent/lga_agent.cc
> @@ -1055,7 +1055,7 @@ SaAisErrorT LogAgent::HandleLogRecord(const
> SaLogRecordT* logRecord,
>  ais_rc = SA_AIS_ERR_INVALID_PARAM;
>  return ais_rc;
>}
> -  if (strlen(logSvcUsrName) >= kOsafMaxDnLength) {
> +  if (strlen(logSvcUsrChars) > kOsafMaxDnLength) {
>  TRACE("SA_AMF_COMPONENT_NAME is too long");
>  ais_rc = SA_AIS_ERR_INVALID_PARAM;
>  return ais_rc;
> @@ -,7 +,7 @@ SaAisErrorT
> LogAgent::saLogWriteLogAsync(SaLogStreamHandleT logStreamHandle,
>lgsv_msg_t msg;
>SaAisErrorT ais_rc = SA_AIS_OK;
>lgsv_write_log_async_req_t* write_param;
> -  char logSvcUsrName[kOsafMaxDnLength] = {0};
> +  char logSvcUsrName[kOsafMaxDnLength + 1] = {0};
>bool is_locked = false;
>SaNameT tmpSvcUsrName;
>bool cUpdated, sUpdated;
> diff --git a/src/log/agent/lga_mds.cc b/src/log/agent/lga_mds.cc
> index 78bd7e1ef..374cb59a0 100644
> --- a/src/log/agent/lga_mds.cc
> +++ b/src/log/agent/lga_mds.cc
> @@ -111,6 +111,26 @@ static uint32_t lga_enc_finalize_msg(NCS_UBAID
> *uba, lgsv_msg_t *msg) {
>return total_bytes;
>  }
> 
> +static uint32_t encodeSaNameT(NCS_UBAID *uba, uint8_t *p8, SaNameT
> *name)
> +{
> +  uint32_t total_bytes = 0;
> +  p8 = ncs_enc_reserve_space(uba, 2);
> +  if (!p8) {
> +TRACE("Could not reserve space");
> +return 0;
> +  }
> +
> +  SaConstStringT value = osaf_extended_name_borrow(name);
> +  size_t length = strlen(value);
> +  ncs_encode_16bit(, length);
> +  ncs_enc_claim_space(uba, 2);
> +  total_bytes += 2;
> +  ncs_encode_n_octets_in_uba(
> +  uba, reinterpret_cast(const_cast(value)),
length);
> +  total_bytes += length;
> +  return total_bytes;
> +}
> +
> 
> /*
> ***
>Name  : lga_enc_lstr_open_sync_msg
> 
> @@ -126,30 +146,28 @@ static uint32_t lga_enc_finalize_msg(NCS_UBAID
> *uba, lgsv_msg_t *msg) {
>  static uint32_t lga_enc_lstr_open_sync_msg(NCS_UBAID *uba, lgsv_msg_t
> *msg) {
>int len;
>uint8_t *p8;
> -  uint32_t total_bytes = 0;
> +  uint32_t total_bytes = 0, bytes;
>lgsv_stream_open_req_t *param = 
> >info.api_info.param.lstr_open_sync;
> 
>TRACE_ENTER();
>osafassert(uba != nullptr);
> 
>// Encode the contents
> -  p8 = ncs_enc_reserve_space(uba, 6);
> +  p8 = ncs_enc_reserve_space(uba, 4);
>if (!p8) {
>  TRACE("p8 nullptr!!!");
>  return 0;
>}
> 
> -  SaConstStringT value = osaf_extended_name_borrow(
> >lstr_name);
> -  size_t length = strlen(value);
>ncs_encode_32bit(, param->client_id);
> -  ncs_encode_16bit(, length);
> -  ncs_enc_claim_space(uba, 6);
> -  total_bytes += 6;
> +  ncs_enc_claim_space(uba, 4);
> +  total_bytes += 4;
> 
> -  // Encode log stream name
> -  ncs_encode_n_octets_in_uba(
> -  uba, reinterpret_cast(const_cast(value)),
length);
> -  total_bytes += length;
> +  // Encode logStreamName
> +  if ((bytes = encodeSaNameT(uba, p8, >lstr_name)) == 0) {
> +goto done;
> +  }
> +  total_bytes += bytes;
> 
>// Encode logFileName if initiated
>p8 = ncs_enc_reserve_space(uba, 2);
> @@ -306,43 +324,30 @@ static uint32_t
> lga_enc_lstr_close_msg(NCS_UBAID *uba, lgsv_msg_t *msg) {
>  static uint32_t lga_enc_write_ntf_header(NCS_UBAID *uba,
>   const SaLogNtfLogHeaderT

Re: [devel] [PATCH 1/1] log: fix log supported maximum 2047 characters for long DN [#2525]

[A V Mahesh]

Hi Canh ,

ACK , Not tested.

-AVM


On 7/12/2017 1:40 PM, Canh Van Truong wrote:

Currently, log support maximum 2047 characters for long DN. it should support
maximum 2048 characters.

The patch also fixes to check SA_AMF_COMPONENT_NAME maximum 2048 characters
and refactor encode and decode for SaNameT to process its name in one place
---
  src/log/agent/lga_agent.cc   |   4 +-
  src/log/agent/lga_mds.cc |  99 ++--
  src/log/agent/lga_util.cc|   5 +-
  src/log/apitest/tet_log_longDN.c | 131 +++--
  src/log/logd/lgs_mds.cc  | 137 +++
  src/log/logd/lgs_recov.cc|   6 +-
  src/log/logd/lgs_util.cc |   5 +-
  7 files changed, 210 insertions(+), 177 deletions(-)

diff --git a/src/log/agent/lga_agent.cc b/src/log/agent/lga_agent.cc
index e75415763..d5c7c9430 100644
--- a/src/log/agent/lga_agent.cc
+++ b/src/log/agent/lga_agent.cc
@@ -1055,7 +1055,7 @@ SaAisErrorT LogAgent::HandleLogRecord(const SaLogRecordT* 
logRecord,
  ais_rc = SA_AIS_ERR_INVALID_PARAM;
  return ais_rc;
}
-  if (strlen(logSvcUsrName) >= kOsafMaxDnLength) {
+  if (strlen(logSvcUsrChars) > kOsafMaxDnLength) {
  TRACE("SA_AMF_COMPONENT_NAME is too long");
  ais_rc = SA_AIS_ERR_INVALID_PARAM;
  return ais_rc;
@@ -,7 +,7 @@ SaAisErrorT 
LogAgent::saLogWriteLogAsync(SaLogStreamHandleT logStreamHandle,
lgsv_msg_t msg;
SaAisErrorT ais_rc = SA_AIS_OK;
lgsv_write_log_async_req_t* write_param;
-  char logSvcUsrName[kOsafMaxDnLength] = {0};
+  char logSvcUsrName[kOsafMaxDnLength + 1] = {0};
bool is_locked = false;
SaNameT tmpSvcUsrName;
bool cUpdated, sUpdated;
diff --git a/src/log/agent/lga_mds.cc b/src/log/agent/lga_mds.cc
index 78bd7e1ef..374cb59a0 100644
--- a/src/log/agent/lga_mds.cc
+++ b/src/log/agent/lga_mds.cc
@@ -111,6 +111,26 @@ static uint32_t lga_enc_finalize_msg(NCS_UBAID *uba, 
lgsv_msg_t *msg) {
return total_bytes;
  }
  
+static uint32_t encodeSaNameT(NCS_UBAID *uba, uint8_t *p8, SaNameT *name)

+{
+  uint32_t total_bytes = 0;
+  p8 = ncs_enc_reserve_space(uba, 2);
+  if (!p8) {
+TRACE("Could not reserve space");
+return 0;
+  }
+
+  SaConstStringT value = osaf_extended_name_borrow(name);
+  size_t length = strlen(value);
+  ncs_encode_16bit(, length);
+  ncs_enc_claim_space(uba, 2);
+  total_bytes += 2;
+  ncs_encode_n_octets_in_uba(
+  uba, reinterpret_cast(const_cast(value)), length);
+  total_bytes += length;
+  return total_bytes;
+}
+
  /
Name  : lga_enc_lstr_open_sync_msg
  
@@ -126,30 +146,28 @@ static uint32_t lga_enc_finalize_msg(NCS_UBAID *uba, lgsv_msg_t *msg) {

  static uint32_t lga_enc_lstr_open_sync_msg(NCS_UBAID *uba, lgsv_msg_t *msg) {
int len;
uint8_t *p8;
-  uint32_t total_bytes = 0;
+  uint32_t total_bytes = 0, bytes;
lgsv_stream_open_req_t *param = >info.api_info.param.lstr_open_sync;
  
TRACE_ENTER();

osafassert(uba != nullptr);
  
// Encode the contents

-  p8 = ncs_enc_reserve_space(uba, 6);
+  p8 = ncs_enc_reserve_space(uba, 4);
if (!p8) {
  TRACE("p8 nullptr!!!");
  return 0;
}
  
-  SaConstStringT value = osaf_extended_name_borrow(>lstr_name);

-  size_t length = strlen(value);
ncs_encode_32bit(, param->client_id);
-  ncs_encode_16bit(, length);
-  ncs_enc_claim_space(uba, 6);
-  total_bytes += 6;
+  ncs_enc_claim_space(uba, 4);
+  total_bytes += 4;
  
-  // Encode log stream name

-  ncs_encode_n_octets_in_uba(
-  uba, reinterpret_cast(const_cast(value)), length);
-  total_bytes += length;
+  // Encode logStreamName
+  if ((bytes = encodeSaNameT(uba, p8, >lstr_name)) == 0) {
+goto done;
+  }
+  total_bytes += bytes;
  
// Encode logFileName if initiated

p8 = ncs_enc_reserve_space(uba, 2);
@@ -306,43 +324,30 @@ static uint32_t lga_enc_lstr_close_msg(NCS_UBAID *uba, 
lgsv_msg_t *msg) {
  static uint32_t lga_enc_write_ntf_header(NCS_UBAID *uba,
   const SaLogNtfLogHeaderT *ntfLogH) {
uint8_t *p8;
-  uint32_t total_bytes = 0;
+  uint32_t total_bytes = 0, bytes;
  
-  p8 = ncs_enc_reserve_space(uba, 14);

+  p8 = ncs_enc_reserve_space(uba, 12);
if (!p8) {
  TRACE("Could not reserve space");
  return 0;
}
ncs_encode_64bit(, ntfLogH->notificationId);
ncs_encode_32bit(, ntfLogH->eventType);
+  ncs_enc_claim_space(uba, 12);
+  total_bytes += 12;
  
-  SaConstStringT value = osaf_extended_name_borrow(ntfLogH->notificationObject);

-  size_t length = strlen(value);
-
-  ncs_encode_16bit(, length);
-  ncs_enc_claim_space(uba, 14);
-  total_bytes += 14;
  
-  ncs_encode_n_octets_in_uba(

-  uba, reinterpret_cast(const_cast(value)), length);
-  total_bytes += length;
-
-  p8 = ncs_enc_reserve_space(uba, 2);
-  if (!p8) {
-TRACE("Could not reserve space");
+  //