Hi, Find my comments inline started with [Zoran].
-----Original Message----- From: Minh Hon Chau [mailto:minh.c...@dektech.com.au] Sent: den 2 juli 2014 07:50 To: Anders Widell; mathi.naic...@oracle.com; Hans Feldt; praveen.malv...@oracle.com Cc: opensaf-devel@lists.sourceforge.net Subject: [devel] [PATCH 2 of 5] NTF: Adapt NTF common library to support long DNs [#873] osaf/libs/common/ntfsv/Makefile.am | 1 + osaf/libs/common/ntfsv/include/ntfsv_mem.h | 7 + osaf/libs/common/ntfsv/ntfsv_enc_dec.c | 37 ++++-- osaf/libs/common/ntfsv/ntfsv_mem.c | 167 +++++++++++++++++++++++++++- 4 files changed, 191 insertions(+), 21 deletions(-) diff --git a/osaf/libs/common/ntfsv/Makefile.am b/osaf/libs/common/ntfsv/Makefile.am --- a/osaf/libs/common/ntfsv/Makefile.am +++ b/osaf/libs/common/ntfsv/Makefile.am @@ -23,6 +23,7 @@ SUBDIRS = include noinst_LTLIBRARIES = libntfsv_common.la libntfsv_common_la_CPPFLAGS = \ + -DSA_EXTENDED_NAME_SOURCE \ $(AM_CPPFLAGS) \ -I$(top_srcdir)/osaf/libs/common/ntfsv/include diff --git a/osaf/libs/common/ntfsv/include/ntfsv_mem.h b/osaf/libs/common/ntfsv/include/ntfsv_mem.h --- a/osaf/libs/common/ntfsv/include/ntfsv_mem.h +++ b/osaf/libs/common/ntfsv/include/ntfsv_mem.h @@ -97,6 +97,13 @@ extern "C" { void ntfsv_filter_obj_cr_del_free(SaNtfObjectCreateDeleteNotificationFilterT *f); void ntfsv_filter_attr_ch_free(SaNtfAttributeChangeNotificationFilterT *f); + SaAisErrorT ntfsv_sanamet_copy(SaNameT* pDes, SaNameT* pSrc); + bool ntfsv_sanamet_is_valid(const SaNameT* pName); + void ntfsv_sanamet_clone_strptr(SaNameT* pName); + SaStringT ntfs_sanamet_strdup(SaNameT* pName); + size_t ntfs_sanamet_length(const SaNameT* pName); + void ntfs_sanamet_steal(SaStringT value, size_t length, SaNameT* pName); + void ntfs_sanamet_alloc(SaConstStringT value, size_t length, SaNameT* pName); #ifdef __cplusplus } #endif diff --git a/osaf/libs/common/ntfsv/ntfsv_enc_dec.c b/osaf/libs/common/ntfsv/ntfsv_enc_dec.c --- a/osaf/libs/common/ntfsv/ntfsv_enc_dec.c +++ b/osaf/libs/common/ntfsv/ntfsv_enc_dec.c @@ -18,6 +18,8 @@ #include <ncsencdec_pub.h> #include "ntfsv_enc_dec.h" #include "ntfsv_mem.h" +#include "osaf_extended_name.h" +#include "saAis.h" typedef union { uint32_t uint32_val; @@ -334,19 +336,22 @@ static uint32_t decodeSaNtfAttribute(NCS static uint32_t encodeSaNameT(NCS_UBAID *uba, uint8_t *p8, SaNameT *name) { uint32_t rv; - p8 = ncs_enc_reserve_space(uba, 2); if (!p8) { TRACE("ncs_enc_reserve_space failed"); return NCSCC_RC_OUT_OF_MEM; } - if (name->length > SA_MAX_NAME_LENGTH) { - LOG_ER("SaNameT length too long %hd", name->length); - osafassert(0); - } - ncs_encode_16bit(&p8, name->length); - ncs_enc_claim_space(uba, 2); - rv = ncs_encode_n_octets_in_uba(uba, name->value, (uint32_t)name->length); + + if (!ntfsv_sanamet_is_valid(name)) { + LOG_ER("SaNameT is invalid"); + osafassert(0); + } + + SaConstStringT value = osaf_extended_name_borrow(name); + size_t length = ntfs_sanamet_length(name); + ncs_encode_16bit(&p8, length); + ncs_enc_claim_space(uba, 2); + rv = ncs_encode_n_octets_in_uba(uba, (uint8_t*) value, (uint32_t) length); return rv; } @@ -355,14 +360,22 @@ static uint32_t decodeSaNameT(NCS_UBAID uint8_t local_data[2]; uint32_t rv; p8 = ncs_dec_flatten_space(uba, local_data, 2); - name->length = ncs_decode_16bit(&p8); - if (name->length > SA_MAX_NAME_LENGTH) { - LOG_ER("SaNameT length too long: %hd", name->length); + size_t length = ncs_decode_16bit(&p8); + if (length > kMaxDnLength) { + LOG_ER("SaNameT length too long: %zu", length); /* this should not happen */ osafassert(0); } ncs_dec_skip_space(uba, 2); - rv = ncs_decode_n_octets_from_uba(uba, name->value, (uint32_t)name->length); + char* value = (char*) malloc(length + 1); + if (value == NULL) { + LOG_ER("Out of memory"); + /* this should not happen */ + osafassert(0); + } + rv = ncs_decode_n_octets_from_uba(uba, (uint8_t*) value, (uint32_t) length); + value[length] = '\0'; + ntfs_sanamet_steal(value, length, name); return rv; } diff --git a/osaf/libs/common/ntfsv/ntfsv_mem.c b/osaf/libs/common/ntfsv/ntfsv_mem.c --- a/osaf/libs/common/ntfsv/ntfsv_mem.c +++ b/osaf/libs/common/ntfsv/ntfsv_mem.c @@ -18,6 +18,8 @@ #include <saNtf.h> #include <stdlib.h> #include "ntfsv_mem.h" +#include "osaf_extended_name.h" +#include "saAis.h" #include <logtrace.h> void ntfsv_free_header(const SaNtfNotificationHeaderT *notificationHeader) @@ -27,10 +29,14 @@ void ntfsv_free_header(const SaNtfNotifi free(notificationHeader->eventType); if (notificationHeader->eventTime != NULL) free(notificationHeader->eventTime); - if (notificationHeader->notificationObject != NULL) + if (notificationHeader->notificationObject != NULL) { + osaf_extended_name_free(notificationHeader->notificationObject); free(notificationHeader->notificationObject); - if (notificationHeader->notifyingObject != NULL) + } + if (notificationHeader->notifyingObject != NULL) { + osaf_extended_name_free(notificationHeader->notifyingObject); free(notificationHeader->notifyingObject); + } if (notificationHeader->notificationClassId != NULL) free(notificationHeader->notificationClassId); if (notificationHeader->notificationId != NULL) @@ -486,10 +492,10 @@ void ntfsv_copy_ntf_header(SaNtfNotifica *(dest->eventTime) = *(src->eventTime); /* Set Notification Object */ - *(dest->notificationObject) = *(src->notificationObject); + ntfsv_sanamet_copy(dest->notificationObject, src->notificationObject); /* Set Notifying Object */ - *(dest->notifyingObject) = *(src->notifyingObject); + ntfsv_sanamet_copy(dest->notifyingObject, src->notifyingObject); /* set Notification Class Identifier */ dest->notificationClassId->vendorId = src->notificationClassId->vendorId; @@ -954,7 +960,7 @@ SaAisErrorT ntfsv_filter_header_alloc(Sa /* Notification objects */ if (numNotificationObjects != 0) { - header->notificationObjects = (SaNameT *)malloc(numNotificationObjects * sizeof(SaNameT)); + header->notificationObjects = (SaNameT *)calloc(numNotificationObjects, sizeof(SaNameT)); if (header->notificationObjects == NULL) { TRACE_1("Out of memory notificationObjects"); rc = SA_AIS_ERR_NO_MEMORY; @@ -964,7 +970,7 @@ SaAisErrorT ntfsv_filter_header_alloc(Sa /* Notifying objects */ if (numNotifyingObjects != 0) { - header->notifyingObjects = (SaNameT *)malloc(numNotifyingObjects * sizeof(SaNameT)); + header->notifyingObjects = (SaNameT *)calloc(numNotifyingObjects, sizeof(SaNameT)); if (header->notifyingObjects == NULL) { TRACE_1("Out of memory notifyingObjects"); rc = SA_AIS_ERR_NO_MEMORY; @@ -1174,10 +1180,18 @@ error_free: void ntfsv_filter_header_free(SaNtfNotificationFilterHeaderT *header) { + size_t i; + + for (i = 0; i < header->numNotificationObjects; ++i) + osaf_extended_name_free(header->notificationObjects + i); + + for (i = 0; i < header->numNotifyingObjects; ++i) + osaf_extended_name_free(header->notifyingObjects + i); + free(header->eventTypes); - free(header->notificationObjects); - free(header->notifyingObjects); - free(header->notificationClassIds); + free(header->notificationObjects); + free(header->notifyingObjects); + free(header->notificationClassIds); } void ntfsv_filter_sec_alarm_free(SaNtfSecurityAlarmNotificationFilterT *s_filter) @@ -1226,3 +1240,138 @@ void ntfsv_filter_attr_ch_free(SaNtfAttr } } +/** + * @Brief: Copy SaNameT pointed by pSrc to pDes + * + */ +SaAisErrorT ntfsv_sanamet_copy(SaNameT* pDes, SaNameT* pSrc) +{ + SaAisErrorT rc = SA_AIS_OK; + if (osaf_is_an_extended_name(pDes)) { + TRACE("Can not modify the existing longDn object"); + return SA_AIS_ERR_EXIST; + } + if (osaf_is_an_extended_name(pSrc)) { + osaf_extended_name_alloc(osaf_extended_name_borrow(pSrc), pDes); + } else { + /* pSrc is old SaNameT format, @.length could be counted with or + * without null-termination, we need to preserve the @.length, + * just a memcpy here + */ + memcpy(pDes, pSrc, sizeof(SaNameT)); + } + return rc; [Zoran] Minor comment. "rc" is not necessary in the function. "return rc;" can be replaced with "return SA_AIS_OK;", and skip using "rc" +} + +/** + * @Brief: Check SaNameT is a valid formation + * + */ +bool ntfsv_sanamet_is_valid(const SaNameT* pName) +{ + if (!osaf_is_extended_name_valid(pName)) { + LOG_ER("Environment variable SA_ENABLE_EXTENDED_NAMES " + "is not set, or not using extended name api"); + return false; + } + if (osaf_extended_name_length(pName) > kMaxDnLength) { + LOG_ER("Exceeding maximum of extended name length(%u)" + ,kMaxDnLength); + return false; + } + return true; +} + +/** + * @Brief: Clone the string pointer in extended name + * + */ +void ntfsv_sanamet_clone_strptr(SaNameT* pName) +{ + if (osaf_is_an_extended_name(pName)) + osaf_extended_name_alloc(osaf_extended_name_borrow(pName), pName); [Zoran] Please check how this function is used. Seems like that this code leads to a memory leak. Pointer in the extended name is overwritten. I'll give you an example of using this function in another email. +} + +/** + * @Brief: Return the length of string specified in SaNameT type. + * + */ +size_t ntfs_sanamet_length(const SaNameT* pName) +{ + if (osaf_is_an_extended_name(pName)) + return osaf_extended_name_length(pName); + + /* In case of unextended name, the length returned by + * osaf_extended_name_length sometimes is greater than @.length since if + * @.value is not a null-terminated string, and it reads over the + * @.length character(s). Here we need the length specified in @.length + */ [Zoran] This text is not true. osaf_extended_name_length() takes care of non-null-terminated string. If it's not extended name, then osaf_extended_name_length() will return max @.length. Taken from osaf_extended_name_length the code: length = strnlen((const char*) (name->_opaque + 1), length); + size_t length = 0; + memcpy(&length, (char*)pName, 2); [Zoran] This code will not work correct on big endian system. The size of size_t is greater than 16 bits. This is a minor comment, since ntfs_sanamet_length can be replaced osaf_extended_name_length. + osafassert(length < SA_MAX_UNEXTENDED_NAME_LENGTH); + return length; +} + +/** + * @Brief: Allocate the new null-terminated string in SaNameT type, and return + * the pointer of this new string. + * + */ +SaStringT ntfs_sanamet_strdup(SaNameT* pName) +{ + SaStringT pRtr; + SaConstStringT pStr; + + size_t length = 0; + if (osaf_is_an_extended_name(pName)) { + pStr = osaf_extended_name_borrow(pName); + length = strlen(pStr) + 1; + } else { + length = ntfs_sanamet_length(pName); + osafassert(length < SA_MAX_UNEXTENDED_NAME_LENGTH); + pStr = (char*)(pName) + 2; + /* for the case of non null-terminated in @.value */ + if (pStr[length - 1] != '\0') + length++; + + } [Zoran] As I mentioned in the comment before, osaf_extended_name_length() works fine with both extended and non-extended names. There is no need for making double work and making distinguish between extended and non-extended name. osaf_extended_name_* functions should take care of that. The code above can be safely replaced with: length = osaf_extended_name_length(pName) + 1; pStr = osaf_extended_name_borrow(pName); + pRtr = (char*) malloc(length); + if (pRtr == NULL) { + LOG_ER("out of memory"); + return NULL; + } + memcpy(pRtr, pStr, length); [Zoran] memcpy(pRtr, pStr, length - 1); Best regards, Zoran + pRtr[length - 1] = '\0'; + return pRtr; +} + +/** + * @Brief: Create SaNameT input by string @value and @length, + * @value is not freed then + */ +void ntfs_sanamet_alloc(SaConstStringT value, size_t length, SaNameT* pName) +{ + osaf_extended_name_alloc(value, pName); + /* Accept the old SaNameT which's @.length counting the null termination */ + if (!osaf_is_an_extended_name(pName) + && ((ntfs_sanamet_length(pName)+1) == length)) { + memcpy(pName, &length, 2); + } +} + +/** + * @Brief: Create SaNameT input by string @value and @length, + * @value is freed then + */ +void ntfs_sanamet_steal(SaStringT value, size_t length, SaNameT* pName) +{ + /* create pName by string @value */ + osaf_extended_name_free(pName); + osaf_extended_name_steal(value, pName); + + /* Accept the old SaNameT which's @.length counting the null termination */ + if (!osaf_is_an_extended_name(pName) + && ((ntfs_sanamet_length(pName)+1) == length)) { + memcpy(pName, &length, 2); + } +} ------------------------------------------------------------------------------ Open source business process management suite built on Java and Eclipse Turn processes into business applications with Bonita BPM Community Edition Quickly connect people, data, and systems into organized workflows Winner of BOSSIE, CODIE, OW2 and Gartner awards http://p.sf.net/sfu/Bonitasoft _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel ------------------------------------------------------------------------------ Open source business process management suite built on Java and Eclipse Turn processes into business applications with Bonita BPM Community Edition Quickly connect people, data, and systems into organized workflows Winner of BOSSIE, CODIE, OW2 and Gartner awards http://p.sf.net/sfu/Bonitasoft _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
