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
> <canh.v.tru...@dektech.com.au>
> 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;
> @@ -1111,7 +1111,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(&p8, length);
> +  ncs_enc_claim_space(uba, 2);
> +  total_bytes += 2;
> +  ncs_encode_n_octets_in_uba(
> +      uba, reinterpret_cast<uint8_t *>(const_cast<char *>(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 = &msg-
> >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(&param-
> >lstr_name);
> -  size_t length = strlen(value);
>    ncs_encode_32bit(&p8, param->client_id);
> -  ncs_encode_16bit(&p8, 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<uint8_t *>(const_cast<char *>(value)),
length);
> -  total_bytes += length;
> +  // Encode logStreamName
> +  if ((bytes = encodeSaNameT(uba, p8, &param->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(&p8, ntfLogH->notificationId);
>    ncs_encode_32bit(&p8, 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(&p8, length);
> -  ncs_enc_claim_space(uba, 14);
> -  total_bytes += 14;
> 
> -  ncs_encode_n_octets_in_uba(
> -      uba, reinterpret_cast<uint8_t *>(const_cast<char *>(value)),
length);
> -  total_bytes += length;
> -
> -  p8 = ncs_enc_reserve_space(uba, 2);
> -  if (!p8) {
> -    TRACE("Could not reserve space");
> +  // Encode notificationObject
> +  if ((bytes = encodeSaNameT(uba, p8, ntfLogH->notificationObject)) == 0)
{
>      return 0;
>    }
> +  total_bytes += bytes;
> 
> -  SaConstStringT notifObj = osaf_extended_name_borrow(ntfLogH-
> >notifyingObject);
> -  size_t notifLength = strlen(notifObj);
> -  ncs_encode_16bit(&p8, notifLength);
> -  ncs_enc_claim_space(uba, 2);
> -  total_bytes += 2;
> -
> -  ncs_encode_n_octets_in_uba(
> -      uba, reinterpret_cast<uint8_t *>(const_cast<char *>(notifObj)),
> -      notifLength);
> -  total_bytes += notifLength;
> +  // Encode notifyingObject
> +  if ((bytes = encodeSaNameT(uba, p8, ntfLogH->notifyingObject)) == 0) {
> +    return 0;
> +  }
> +  total_bytes += bytes;
> 
>    p8 = ncs_enc_reserve_space(uba, 16);
>    if (!p8) {
> @@ -363,9 +368,9 @@ static uint32_t lga_enc_write_gen_header(
>      NCS_UBAID *uba, const lgsv_write_log_async_req_t *param,
>      const SaLogGenericLogHeaderT *genLogH) {
>    uint8_t *p8;
> -  uint32_t total_bytes = 0;
> +  uint32_t total_bytes = 0, bytes;
> 
> -  p8 = ncs_enc_reserve_space(uba, 10);
> +  p8 = ncs_enc_reserve_space(uba, 8);
>    if (!p8) {
>      TRACE("Could not reserve space");
>      return 0;
> @@ -373,18 +378,14 @@ static uint32_t lga_enc_write_gen_header(
>    ncs_encode_32bit(&p8, 0);
>    ncs_encode_16bit(&p8, 0);
>    ncs_encode_16bit(&p8, 0);
> +  ncs_enc_claim_space(uba, 8);
> +  total_bytes += 8;
> 
> -  SaConstStringT usrName = osaf_extended_name_borrow(param-
> >logSvcUsrName);
> -  size_t nameLength = strlen(usrName) + 1;
> -
> -  ncs_encode_16bit(&p8, nameLength);
> -  ncs_enc_claim_space(uba, 10);
> -  total_bytes += 10;
> -
> -  ncs_encode_n_octets_in_uba(
> -      uba, reinterpret_cast<uint8_t *>(const_cast<char *>(usrName)),
> -      nameLength);
> -  total_bytes += nameLength;
> +  // Encode logSvcUsrName
> +  if ((bytes = encodeSaNameT(uba, p8, param->logSvcUsrName)) == 0) {
> +    return 0;
> +  }
> +  total_bytes += bytes;
> 
>    p8 = ncs_enc_reserve_space(uba, 2);
>    if (!p8) {
> diff --git a/src/log/agent/lga_util.cc b/src/log/agent/lga_util.cc
> index 143de5344..231e9b2bc 100644
> --- a/src/log/agent/lga_util.cc
> +++ b/src/log/agent/lga_util.cc
> @@ -143,8 +143,9 @@ bool lga_is_extended_name_valid(const SaNameT*
> name) {
>    if (name == nullptr) return false;
>    if (osaf_is_extended_name_valid(name) == false) return false;
> 
> -  SaConstStringT str = osaf_extended_name_borrow(name);
> -  if (strlen(str) >= kOsafMaxDnLength) return false;
> +  if (osaf_extended_name_length(name) > kOsafMaxDnLength) {
> +    return false;
> +  }
> 
>    return true;
>  }
> diff --git a/src/log/apitest/tet_log_longDN.c
> b/src/log/apitest/tet_log_longDN.c
> index 33c74a270..a8b6f0eb6 100644
> --- a/src/log/apitest/tet_log_longDN.c
> +++ b/src/log/apitest/tet_log_longDN.c
> @@ -754,21 +754,21 @@ void longDNAt_No_token(void)
>       ais = initLog();
>       if (ais != SA_AIS_OK) {
>               fprintf(stderr, "initLog FAILED: %s\n", saf_error(ais));
> -             rc_validate(ais, SA_AIS_OK);
> +             test_validate(ais, SA_AIS_OK);
>               goto done;
>       }
> 
>       ais = openLog(E_ALARM);
>       if (ais != SA_AIS_OK) {
>               fprintf(stderr, "openLog FAILED: %s\n", saf_error(ais));
> -             rc_validate(ais, SA_AIS_OK);
> +             test_validate(ais, SA_AIS_OK);
>               goto done_init;
>       }
> 
>       ais = saLogSelectionObjectGet(logHandleLd, &selectionObjectLd);
>       if (ais != SA_AIS_OK) {
>               printf("saLogSelectionObjectGet FAILED: %s\n",
> saf_error(ais));
> -             rc_validate(ais, SA_AIS_OK);
> +             test_validate(ais, SA_AIS_OK);
>               goto done_init;
>       }
> 
> @@ -780,7 +780,7 @@ void longDNAt_No_token(void)
>       ais = writeLog();
>       if (ais != SA_AIS_OK) {
>               fprintf(stderr, "saLogOpen FAILED: %s\n", saf_error(ais));
> -             rc_validate(ais, SA_AIS_OK);
> +             test_validate(ais, SA_AIS_OK);
>               goto done_init;
>       }
> 
> @@ -821,21 +821,21 @@ void longDNAt_Ng_token(void)
>       ais = initLog();
>       if (ais != SA_AIS_OK) {
>               fprintf(stderr, "saLogInitialize FAILED: %s\n",
saf_error(ais));
> -             rc_validate(ais, SA_AIS_OK);
> +             test_validate(ais, SA_AIS_OK);
>               goto done;
>       }
> 
>       ais = openLog(E_NOTIF);
>       if (ais != SA_AIS_OK) {
>               fprintf(stderr, "saLogOpen FAILED: %s\n", saf_error(ais));
> -             rc_validate(ais, SA_AIS_OK);
> +             test_validate(ais, SA_AIS_OK);
>               goto done_init;
>       }
> 
>       ais = saLogSelectionObjectGet(logHandleLd, &selectionObjectLd);
>       if (ais != SA_AIS_OK) {
>               printf("saLogSelectionObjectGet FAILED: %s\n",
> saf_error(ais));
> -             rc_validate(ais, SA_AIS_OK);
> +             test_validate(ais, SA_AIS_OK);
>               goto done_init;
>       }
> 
> @@ -847,7 +847,7 @@ void longDNAt_Ng_token(void)
>       ais = writeLog();
>       if (ais != SA_AIS_OK) {
>               fprintf(stderr, "saLogOpen FAILED: %s\n", saf_error(ais));
> -             rc_validate(ais, SA_AIS_OK);
> +             test_validate(ais, SA_AIS_OK);
>               goto done_init;
>       }
> 
> @@ -888,21 +888,21 @@ void longDNAt_Sl_token(void)
>       ais = initLog();
>       if (ais != SA_AIS_OK) {
>               fprintf(stderr, "saLogInitialize FAILED: %s\n",
saf_error(ais));
> -             rc_validate(ais, SA_AIS_OK);
> +             test_validate(ais, SA_AIS_OK);
>               goto done;
>       }
> 
>       ais = openLog(E_SYSTE);
>       if (ais != SA_AIS_OK) {
>               fprintf(stderr, "saLogOpen FAILED: %s\n", saf_error(ais));
> -             rc_validate(ais, SA_AIS_OK);
> +             test_validate(ais, SA_AIS_OK);
>               goto done_init;
>       }
> 
>       ais = saLogSelectionObjectGet(logHandleLd, &selectionObjectLd);
>       if (ais != SA_AIS_OK) {
>               printf("saLogSelectionObjectGet FAILED: %s\n",
> saf_error(ais));
> -             rc_validate(ais, SA_AIS_OK);
> +             test_validate(ais, SA_AIS_OK);
>               goto done_init;
>       }
> 
> @@ -914,7 +914,7 @@ void longDNAt_Sl_token(void)
>       ais = writeLog();
>       if (ais != SA_AIS_OK) {
>               fprintf(stderr, "saLogOpen FAILED: %s\n", saf_error(ais));
> -             rc_validate(ais, SA_AIS_OK);
> +             test_validate(ais, SA_AIS_OK);
>               goto done_init;
>       }
> 
> @@ -962,28 +962,28 @@ void longDN_AppStream(void)
>       ais = initLog();
>       if (ais != SA_AIS_OK) {
>               fprintf(stderr, "saLogInitialize FAILED: %s\n",
saf_error(ais));
> -             rc_validate(ais, SA_AIS_OK);
> +             test_validate(ais, SA_AIS_OK);
>               goto done;
>       }
> 
>       ais = openLog(E_APPLI);
>       if (ais != SA_AIS_OK) {
>               fprintf(stderr, "saLogOpen FAILED: %s\n", saf_error(ais));
> -             rc_validate(ais, SA_AIS_OK);
> +             test_validate(ais, SA_AIS_OK);
>               goto done_init;
>       }
> 
>       ais = saLogSelectionObjectGet(logHandleLd, &selectionObjectLd);
>       if (ais != SA_AIS_OK) {
>               printf("saLogSelectionObjectGet FAILED: %s\n",
> saf_error(ais));
> -             rc_validate(ais, SA_AIS_OK);
> +             test_validate(ais, SA_AIS_OK);
>               goto done_init;
>       }
> 
>       ais = writeLog();
>       if (ais != SA_AIS_OK) {
>               fprintf(stderr, "saLogOpen FAILED: %s\n", saf_error(ais));
> -             rc_validate(ais, SA_AIS_OK);
> +             test_validate(ais, SA_AIS_OK);
>               goto done_init;
>       }
> 
> @@ -1064,31 +1064,31 @@ void longDN_No_Over_MaxDn(void)
>       ais = initLog();
>       if (ais != SA_AIS_OK) {
>               fprintf(stderr, "initLog FAILED: %s\n", saf_error(ais));
> -             rc_validate(ais, SA_AIS_OK);
> +             test_validate(ais, SA_AIS_OK);
>               goto done;
>       }
> 
>       ais = openLog(E_ALARM);
>       if (ais != SA_AIS_OK) {
>               fprintf(stderr, "openLog FAILED: %s\n", saf_error(ais));
> -             rc_validate(ais, SA_AIS_OK);
> +             test_validate(ais, SA_AIS_OK);
>               goto done_init;
>       }
> 
>       ais = saLogSelectionObjectGet(logHandleLd, &selectionObjectLd);
>       if (ais != SA_AIS_OK) {
>               printf("saLogSelectionObjectGet FAILED: %s\n",
> saf_error(ais));
> -             rc_validate(ais, SA_AIS_OK);
> +             test_validate(ais, SA_AIS_OK);
>               goto done_init;
>       }
> 
>       // Long DN in @No token
> -     char notificationObj[kOsafMaxDnLength + 1] = {0};
> -     memset(notificationObj, 'X', sizeof(notificationObj) - 1);
> +     char notificationObj[kOsafMaxDnLength + 2] = {0};
> +     memset(notificationObj, 'X', kOsafMaxDnLength + 1);
>       saAisNameLend(notificationObj, &notificationObjLd);
> 
>       ais = writeLog();
> -     rc_validate(ais, SA_AIS_ERR_INVALID_PARAM);
> +     test_validate(ais, SA_AIS_ERR_INVALID_PARAM);
> 
>  // End testing. Close handles and restore data
>  done_init:
> @@ -1124,31 +1124,31 @@ void longDN_Ng_Over_MaxDn(void)
>       ais = initLog();
>       if (ais != SA_AIS_OK) {
>               fprintf(stderr, "saLogInitialize FAILED: %s\n",
saf_error(ais));
> -             rc_validate(ais, SA_AIS_OK);
> +             test_validate(ais, SA_AIS_OK);
>               goto done;
>       }
> 
>       ais = openLog(E_NOTIF);
>       if (ais != SA_AIS_OK) {
>               fprintf(stderr, "saLogOpen FAILED: %s\n", saf_error(ais));
> -             rc_validate(ais, SA_AIS_OK);
> +             test_validate(ais, SA_AIS_OK);
>               goto done_init;
>       }
> 
>       ais = saLogSelectionObjectGet(logHandleLd, &selectionObjectLd);
>       if (ais != SA_AIS_OK) {
>               printf("saLogSelectionObjectGet FAILED: %s\n",
> saf_error(ais));
> -             rc_validate(ais, SA_AIS_OK);
> +             test_validate(ais, SA_AIS_OK);
>               goto done_init;
>       }
> 
>       // Long DN in @No token
> -     char notifyingObj[kOsafMaxDnLength + 1] = {0};
> -     memset(notifyingObj, 'Y', sizeof(notifyingObj) - 1);
> +     char notifyingObj[kOsafMaxDnLength + 2] = {0};
> +     memset(notifyingObj, 'Y', kOsafMaxDnLength + 1);
>       saAisNameLend(notifyingObj, &notifyingObjLd);
> 
>       ais = writeLog();
> -     rc_validate(ais, SA_AIS_ERR_INVALID_PARAM);
> +     test_validate(ais, SA_AIS_ERR_INVALID_PARAM);
> 
>  // End testing. Close handles and restore data
>  done_init:
> @@ -1184,31 +1184,31 @@ void longDN_Sl_Over_MaxDn(void)
>       ais = initLog();
>       if (ais != SA_AIS_OK) {
>               fprintf(stderr, "saLogInitialize FAILED: %s\n",
saf_error(ais));
> -             rc_validate(ais, SA_AIS_OK);
> +             test_validate(ais, SA_AIS_OK);
>               goto done;
>       }
> 
>       ais = openLog(E_SYSTE);
>       if (ais != SA_AIS_OK) {
>               fprintf(stderr, "saLogOpen FAILED: %s\n", saf_error(ais));
> -             rc_validate(ais, SA_AIS_OK);
> +             test_validate(ais, SA_AIS_OK);
>               goto done_init;
>       }
> 
>       ais = saLogSelectionObjectGet(logHandleLd, &selectionObjectLd);
>       if (ais != SA_AIS_OK) {
>               printf("saLogSelectionObjectGet FAILED: %s\n",
> saf_error(ais));
> -             rc_validate(ais, SA_AIS_OK);
> +             test_validate(ais, SA_AIS_OK);
>               goto done_init;
>       }
> 
>       // Long DN in @Sl token
> -     char serviceName[kOsafMaxDnLength + 1] = {0};
> -     memset(serviceName, 'Z', sizeof(serviceName) - 1);
> +     char serviceName[kOsafMaxDnLength + 2] = {0};
> +     memset(serviceName, 'Z', kOsafMaxDnLength + 1);
>       saAisNameLend(serviceName, &logSvcUsrNameLd);
> 
>       ais = writeLog();
> -     rc_validate(ais, SA_AIS_ERR_INVALID_PARAM);
> +     test_validate(ais, SA_AIS_ERR_INVALID_PARAM);
> 
>  // End testing. Close handles and restore data
>  done_init:
> @@ -1283,6 +1283,66 @@ void longDNIn_AppStreamDN_ButNoF(void)
>       rc_validate(WEXITSTATUS(rc), 0);
>  }
> 
> +//>>
> +// UC1: Write an log records with length of `notificationObject`
> +// is kOsafMaxDnLength (2048) using Log API.
> +//<<
> +void longDN_No_MaxDn(void)
> +{
> +     int rc;
> +     SaAisErrorT ais;
> +
> +     rc = backupData(E_ALARM);
> +     if (rc != 0) {
> +             fprintf(stderr, "Backup data failed\n");
> +             // Not report test failed as can use default data to
restore.
> +             // rc_validate(WEXITSTATUS(rc), 0);
> +             // return;
> +     }
> +
> +     rc = setUpTestEnv(E_ALARM);
> +     if (rc != 0) {
> +             fprintf(stderr, "set up env failed\n");
> +             rc_validate(WEXITSTATUS(rc), 0);
> +             goto done;
> +     }
> +
> +     ais = initLog();
> +     if (ais != SA_AIS_OK) {
> +             fprintf(stderr, "initLog FAILED: %s\n", saf_error(ais));
> +             test_validate(ais, SA_AIS_OK);
> +             goto done;
> +     }
> +
> +     ais = openLog(E_ALARM);
> +     if (ais != SA_AIS_OK) {
> +             fprintf(stderr, "openLog FAILED: %s\n", saf_error(ais));
> +             test_validate(ais, SA_AIS_OK);
> +             goto done_init;
> +     }
> +
> +     ais = saLogSelectionObjectGet(logHandleLd, &selectionObjectLd);
> +     if (ais != SA_AIS_OK) {
> +             printf("saLogSelectionObjectGet FAILED: %s\n",
> saf_error(ais));
> +             test_validate(ais, SA_AIS_OK);
> +             goto done_init;
> +     }
> +
> +     // Long DN in @No token
> +     char notificationObj[kOsafMaxDnLength + 1] = {0};
> +     memset(notificationObj, 'X', kOsafMaxDnLength);
> +     saAisNameLend(notificationObj, &notificationObjLd);
> +
> +     ais = writeLog();
> +     test_validate(ais, SA_AIS_OK);
> +
> +// End testing. Close handles and restore data
> +done_init:
> +     endLog();
> +done:
> +     restoreData(E_ALARM);
> +}
> +
>  /*
>   * Suite #13
>   */
> @@ -1317,4 +1377,7 @@ __attribute__((constructor)) static void
> longDN_constructor(void)
>       test_case_add(
>           13, longDNIn_AppStreamDN_ButNoF,
>           "Write a log record to long DN runtime app using saflogger, but
no
> f option");
> +     test_case_add(
> +         13, longDN_No_MaxDn,
> +         "Write a log record with notificationObj (@No) max length");
>  }
> diff --git a/src/log/logd/lgs_mds.cc b/src/log/logd/lgs_mds.cc
> index 4b2cff20e..a835b878a 100644
> --- a/src/log/logd/lgs_mds.cc
> +++ b/src/log/logd/lgs_mds.cc
> @@ -109,6 +109,35 @@ static uint32_t dec_finalize_msg(NCS_UBAID *uba,
> lgsv_msg_t *msg) {
>    return rc;
>  }
> 
> +static uint32_t decodeSaNameT(NCS_UBAID *uba, uint8_t *p8, SaNameT
> *name)
> +{
> +  uint8_t local_data[2];
> +  p8 = ncs_dec_flatten_space(uba, local_data, 2);
> +  // Decode the length of name
> +  size_t length = ncs_decode_16bit(&p8);
> +  if (length > kOsafMaxDnLength) {
> +    LOG_WA("SaNameT length too long: %zu", length);
> +    return NCSCC_RC_FAILURE;
> +  }
> +  ncs_dec_skip_space(uba, 2);
> +
> +  // This allocated memory have to be freed in main thread
> +  char *value = static_cast<char *>(calloc(1, length + 1));
> +  if (value == NULL) {
> +    LOG_ER("Fail to allocated memory");
> +    return NCSCC_RC_FAILURE;
> +  }
> +
> +  // Decode name
> +  ncs_decode_n_octets_from_uba(uba, reinterpret_cast<uint8_t *>(value),
> +                               static_cast<uint32_t>(length));
> +
> +  value[length] = '\0';
> +  osaf_extended_name_clear(name);
> +  osaf_extended_name_steal(value, name);
> +  return NCSCC_RC_SUCCESS;
> +}
> +
> 
> /*************************************************************
> ***************
>    Name          : dec_lstr_open_sync_msg
> 
> @@ -127,7 +156,6 @@ static uint32_t dec_lstr_open_sync_msg(NCS_UBAID
> *uba, lgsv_msg_t *msg) {
>    uint32_t rc = NCSCC_RC_SUCCESS;
>    lgsv_stream_open_req_t *param = &msg-
> >info.api_info.param.lstr_open_sync;
>    uint8_t local_data[256];
> -  char *str_name = NULL;
> 
>    TRACE_ENTER();
>    // To make it safe when using free()
> @@ -140,29 +168,13 @@ static uint32_t
> dec_lstr_open_sync_msg(NCS_UBAID *uba, lgsv_msg_t *msg) {
>    param->client_id = ncs_decode_32bit(&p8);
>    ncs_dec_skip_space(uba, 4);
> 
> -  /* log stream name length */
> -  p8 = ncs_dec_flatten_space(uba, local_data, 2);
> -  size_t length = ncs_decode_16bit(&p8);
> -  ncs_dec_skip_space(uba, 2);
> -
> -  if (length >= kOsafMaxDnLength) {
> -    TRACE("%s - lstr_name too long", __FUNCTION__);
> -    rc = NCSCC_RC_FAILURE;
> -    goto done;
> -  }
> 
> -  /* log stream name */
> -  str_name = static_cast<char *>(calloc(1, length + 1));
> -  if (str_name == NULL) {
> -    LOG_ER("Fail to allocated memory - str_name");
> -    rc = NCSCC_RC_FAILURE;
> +  // Decode logStreamName
> +  rc = decodeSaNameT(uba, p8, &param->lstr_name);
> +  if (rc == NCSCC_RC_FAILURE) {
> +    LOG_WA("Decode logStreamName FAILED");
>      goto done;
>    }
> -  ncs_decode_n_octets_from_uba(uba, reinterpret_cast<uint8_t
> *>(str_name),
> -                               static_cast<uint32_t>(length));
> -  osaf_extended_name_clear(&param->lstr_name);
> -  /* This allocated memory must be freed in proc_stream_open_msg
> @lgs_evt */
> -  osaf_extended_name_alloc(str_name, &param->lstr_name);
> 
>    /* log file name */
>    p8 = ncs_dec_flatten_space(uba, local_data, 2);
> @@ -249,8 +261,6 @@ done_err:
>    free(param->logFileFmt);
> 
>  done:
> -  free(str_name);
> -  str_name = NULL;
>    TRACE_8("LGSV_STREAM_OPEN_REQ");
>    return rc;
>  }
> @@ -287,18 +297,17 @@ static uint32_t
> dec_write_ntf_log_header(NCS_UBAID *uba,
>                                           SaLogNtfLogHeaderT *const
ntfLogH) {
>    uint8_t *p8;
>    uint8_t local_data[1024];
> -  size_t notificationL, notifyingL;
>    uint32_t rc = NCSCC_RC_SUCCESS;
> -  char *notificationObj = NULL;
> -  char *notifyingObj = NULL;
> 
>    ntfLogH->notificationObject = NULL;
>    ntfLogH->notifyingObject = NULL;
>    ntfLogH->notificationClassId = NULL;
> 
> -  p8 = ncs_dec_flatten_space(uba, local_data, 14);
> +  p8 = ncs_dec_flatten_space(uba, local_data, 12);
>    ntfLogH->notificationId = ncs_decode_64bit(&p8);
>    ntfLogH->eventType =
> static_cast<SaNtfEventTypeT>(ncs_decode_32bit(&p8));
> +  ncs_dec_skip_space(uba, 12);
> +
> 
>    ntfLogH->notificationObject =
>        static_cast<SaNameT *>(malloc(sizeof(SaNameT) + 1));
> @@ -308,25 +317,12 @@ static uint32_t
> dec_write_ntf_log_header(NCS_UBAID *uba,
>      goto done;
>    }
> 
> -  notificationL = ncs_decode_16bit(&p8);
> -  if (kOsafMaxDnLength <= notificationL) {
> -    LOG_WA("notificationObject length is so long (max: %d)",
> kOsafMaxDnLength);
> -    rc = NCSCC_RC_FAILURE;
> -    goto done;
> -  }
> -  ncs_dec_skip_space(uba, 14);
> -
> -  notificationObj = static_cast<char *>(calloc(1, notificationL + 1));
> -  if (notificationObj == NULL) {
> -    LOG_WA("Fail to allocated memory - notificationObj");
> -    rc = NCSCC_RC_FAILURE;
> +  // Decode notificationObject
> +  rc = decodeSaNameT(uba, p8, ntfLogH->notificationObject);
> +  if (rc == NCSCC_RC_FAILURE) {
> +    LOG_WA("Decode notificationObject FAILED");
>      goto done;
>    }
> -  ncs_decode_n_octets_from_uba(uba,
> -                               reinterpret_cast<uint8_t
*>(notificationObj),
> -                               static_cast<uint32_t>(notificationL));
> -  osaf_extended_name_clear(ntfLogH->notificationObject);
> -  osaf_extended_name_alloc(notificationObj, ntfLogH->notificationObject);
> 
>    ntfLogH->notifyingObject =
>        static_cast<SaNameT *>(malloc(sizeof(SaNameT) + 1));
> @@ -336,25 +332,12 @@ static uint32_t
> dec_write_ntf_log_header(NCS_UBAID *uba,
>      goto done;
>    }
> 
> -  p8 = ncs_dec_flatten_space(uba, local_data, 2);
> -  notifyingL = ncs_decode_16bit(&p8);
> -  ncs_dec_skip_space(uba, 2);
> -
> -  if (kOsafMaxDnLength <= notifyingL) {
> -    LOG_WA("notifyingObject is so long (max: %d)", kOsafMaxDnLength);
> -    rc = NCSCC_RC_FAILURE;
> -    goto done;
> -  }
> -  notifyingObj = static_cast<char *>(calloc(1, notifyingL + 1));
> -  if (notifyingObj == NULL) {
> -    LOG_WA("Fail to allocated memory - notifyingObj");
> -    rc = NCSCC_RC_FAILURE;
> +  // Decode notifyingObject
> +  rc = decodeSaNameT(uba, p8, ntfLogH->notifyingObject);
> +  if (rc == NCSCC_RC_FAILURE) {
> +    LOG_WA("Decode notifyingObject FAILED");
>      goto done;
>    }
> -  ncs_decode_n_octets_from_uba(uba, reinterpret_cast<uint8_t
> *>(notifyingObj),
> -                               static_cast<uint32_t>(notifyingL));
> -  osaf_extended_name_clear(ntfLogH->notifyingObject);
> -  osaf_extended_name_alloc(notifyingObj, ntfLogH->notifyingObject);
> 
>    ntfLogH->notificationClassId =
>        static_cast<SaNtfClassIdT *>(malloc(sizeof(SaNtfClassIdT)));
> @@ -371,8 +354,6 @@ static uint32_t
> dec_write_ntf_log_header(NCS_UBAID *uba,
>    ncs_dec_skip_space(uba, 16);
> 
>  done:
> -  free(notificationObj);
> -  free(notifyingObj);
>    TRACE("%s - rc = %d", __func__, rc);
>    return rc;
>  }
> @@ -381,9 +362,7 @@ static uint32_t dec_write_gen_log_header(
>      NCS_UBAID *uba, SaLogGenericLogHeaderT *const genLogH) {
>    uint8_t *p8;
>    uint8_t local_data[1024];
> -  size_t svcLength;
>    uint32_t rc = NCSCC_RC_SUCCESS;
> -  char *logSvcUsrName = NULL;
> 
>    genLogH->notificationClassId = NULL;
>    genLogH->logSvcUsrName = NULL;
> @@ -396,42 +375,30 @@ static uint32_t dec_write_gen_log_header(
>      goto done;
>    }
> 
> -  p8 = ncs_dec_flatten_space(uba, local_data, 10);
> +  p8 = ncs_dec_flatten_space(uba, local_data, 8);
>    genLogH->notificationClassId->vendorId = ncs_decode_32bit(&p8);
>    genLogH->notificationClassId->majorId = ncs_decode_16bit(&p8);
>    genLogH->notificationClassId->minorId = ncs_decode_16bit(&p8);
> +  ncs_dec_skip_space(uba, 8);
> 
> -  svcLength = ncs_decode_16bit(&p8);
> -  if (kOsafMaxDnLength <= svcLength) {
> -    LOG_WA("logSvcUsrName too big (max: %d)", kOsafMaxDnLength);
> -    rc = NCSCC_RC_FAILURE;
> -    goto done;
> -  }
> -  logSvcUsrName = static_cast<char *>(malloc(svcLength + 1));
> -  if (logSvcUsrName == NULL) {
> -    LOG_WA("malloc FAILED");
> -    rc = NCSCC_RC_FAILURE;
> -    goto done;
> -  }
>    genLogH->logSvcUsrName = static_cast<SaNameT
> *>(malloc(sizeof(SaNameT) + 1));
>    if (genLogH->logSvcUsrName == NULL) {
>      LOG_WA("malloc FAILED");
>      rc = NCSCC_RC_FAILURE;
>      goto done;
>    }
> -  ncs_dec_skip_space(uba, 10);
> -  ncs_decode_n_octets_from_uba(uba, reinterpret_cast<uint8_t
> *>(logSvcUsrName),
> -                               static_cast<uint32_t>(svcLength));
> -  osaf_extended_name_clear(const_cast<SaNameT *>(genLogH-
> >logSvcUsrName));
> -  osaf_extended_name_alloc(logSvcUsrName,
> -                           const_cast<SaNameT
*>(genLogH->logSvcUsrName));
> +  // Decode logSvcUsrName
> +  rc = decodeSaNameT(uba, p8, const_cast<SaNameT *>(genLogH-
> >logSvcUsrName));
> +  if (rc == NCSCC_RC_FAILURE) {
> +    LOG_WA("Decode logSvcUsrName FAILED");
> +    goto done;
> +  }
> 
>    p8 = ncs_dec_flatten_space(uba, local_data, 2);
>    genLogH->logSeverity = ncs_decode_16bit(&p8);
>    ncs_dec_skip_space(uba, 2);
> 
>  done:
> -  free(logSvcUsrName);
>    TRACE("%s - rc = %d", __func__, rc);
>    return rc;
>  }
> diff --git a/src/log/logd/lgs_recov.cc b/src/log/logd/lgs_recov.cc
> index 8f9f33ccb..c1e5a0c1f 100644
> --- a/src/log/logd/lgs_recov.cc
> +++ b/src/log/logd/lgs_recov.cc
> @@ -109,7 +109,7 @@ int log_rtobj_list_add(const std::string &dn_str) {
>    }
> 
>    /* Save dn string */
> -  len = dn_str.size() + 1; /* Including '\0' */
> +  len = dn_str.size();
>    if (len > kOsafMaxDnLength) {
>      /* Should never happen */
>      LOG_WA("%s\tToo long dn string!", __FUNCTION__);
> @@ -117,7 +117,7 @@ int log_rtobj_list_add(const std::string &dn_str) {
>      goto done;
>    }
> 
> -  str_ptr = static_cast<char *>(calloc(1, len));
> +  str_ptr = static_cast<char *>(calloc(1, len + 1));
>    if (str_ptr == NULL) {
>      LOG_WA("%s\tcalloc Fail", __FUNCTION__);
>      rc = -1;
> @@ -336,7 +336,7 @@ int lgs_restore_one_app_stream(const std::string
> &stream_name,
>      rc_out = -1;
>      goto done;
>    }
> -  if (stream_name.size() >= kOsafMaxDnLength) {
> +  if (stream_name.size() > kOsafMaxDnLength) {
>      TRACE("Log stream name \"%s\" is truncated", stream_name.c_str());
>      rc_out = -1;
>      goto done;
> diff --git a/src/log/logd/lgs_util.cc b/src/log/logd/lgs_util.cc
> index 5ba6257a2..5586cf177 100644
> --- a/src/log/logd/lgs_util.cc
> +++ b/src/log/logd/lgs_util.cc
> @@ -829,8 +829,9 @@ bool lgs_is_extended_name_valid(const SaNameT
> *name) {
>    if (name == NULL) return false;
>    if (osaf_is_extended_name_valid(name) == false) return false;
> 
> -  SaConstStringT str = osaf_extended_name_borrow(name);
> -  if (strlen(str) >= kOsafMaxDnLength) return false;
> +  if (osaf_extended_name_length(name) > kOsafMaxDnLength) {
> +    return false;
> +  }
> 
>    return true;
>  }
> --
> 2.13.0



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to