Please find my comments inline Thanks, Minh On 7/4/2014 9:48 PM, Zoran Milinkovic wrote: > Hi, > > Find my comments on ntfsv_sanamet_clone_strptr cases inline. > > -----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 1 of 5] NTF: Adapt NTF API to support long DNs [#873] > > osaf/libs/agents/saf/ntfa/Makefile.am | 1 + > osaf/libs/agents/saf/ntfa/ntfa_api.c | 25 ++++++++++++++++--------- > osaf/libs/saf/libSaNtf/Makefile.am | 1 + > 3 files changed, 18 insertions(+), 9 deletions(-) > > > diff --git a/osaf/libs/agents/saf/ntfa/Makefile.am > b/osaf/libs/agents/saf/ntfa/Makefile.am > --- a/osaf/libs/agents/saf/ntfa/Makefile.am > +++ b/osaf/libs/agents/saf/ntfa/Makefile.am > @@ -25,6 +25,7 @@ noinst_HEADERS = \ > noinst_LTLIBRARIES = libntfa.la > > libntfa_la_CPPFLAGS = \ > + -DSA_EXTENDED_NAME_SOURCE \ > $(AM_CPPFLAGS) \ > -I$(top_srcdir)/osaf/libs/common/ntfsv/include > > diff --git a/osaf/libs/agents/saf/ntfa/ntfa_api.c > b/osaf/libs/agents/saf/ntfa/ntfa_api.c > --- a/osaf/libs/agents/saf/ntfa/ntfa_api.c > +++ b/osaf/libs/agents/saf/ntfa/ntfa_api.c > @@ -18,7 +18,8 @@ > #include <string.h> > #include "ntfa.h" > #include "ntfsv_mem.h" > - > +#include "osaf_extended_name.h" > +#include "saAis.h" > #define NCS_SAF_MIN_ACCEPT_TIME 10 > #define NTFS_WAIT_TIME 1000 > > @@ -304,9 +305,10 @@ static SaAisErrorT checkSecurityAlarmFil > static SaAisErrorT checkHeader(SaNtfNotificationHeaderT *nh) > { > int i =0; > - > - if (nh->notificationObject->length > SA_MAX_NAME_LENGTH || > nh->notifyingObject->length > SA_MAX_NAME_LENGTH) { > - TRACE_1("SaNameT length too big"); > + > + if (!ntfsv_sanamet_is_valid(nh->notificationObject) || > + !ntfsv_sanamet_is_valid(nh->notifyingObject)) { > + TRACE_1("SaNameT is invaild"); > return SA_AIS_ERR_INVALID_PARAM; > } > > @@ -517,12 +519,17 @@ static SaAisErrorT fillSendStruct(SaNtfN > /* nodificationId set to zero means that this is a new notification */ > /* and not an sync message send from the server. */ > *(notificationHeader->notificationId) = 0; > + > + /* For long dn object, clone the string pointer */ > + ntfsv_sanamet_clone_strptr(notificationHeader->notificationObject); > > [Zoran] Looks like ntfsv_sanamet_clone_strptr makes memory leak here for long > dn ??? Why does long dn need to be cloned ? > > - if (notificationHeader->notifyingObject->length == 0) { > - notificationHeader->notifyingObject->length = > notificationHeader->notificationObject->length; > - (void)memcpy(notificationHeader->notifyingObject->value, > notificationHeader->notificationObject->value, > - notificationHeader->notifyingObject->length); > - } > + /* Copy notificationObject to notifyingObject if it's empty */ > + if (osaf_is_extended_name_empty(notificationHeader->notifyingObject)) > + rc = ntfsv_sanamet_copy(notificationHeader->notifyingObject, > + > notificationHeader->notificationObject); > + else /* Not empty and it's long dn object, clone the string pointer > */ > + ntfsv_sanamet_clone_strptr(notificationHeader->notifyingObject); > > [Zoran] Another case of using ntfsv_sanamet_clone_strptr, and same questions > as above. [Minh]: the logic here is copying notificationObject to notifyingObject if notifyingObject is empty. The ntfs_free_header will free these objects, but it can not know the strptr long dn in notifyingObject made from the app or from the ntf api (unless new flag). So the idea is cloning the long dn. And the app, who allocates the long dn string and use saAisNameLend to create SaNameT, will be reponsible to deallocate this string. This is true to "Memory allocation and deallocation, Rule 1" I think > Best regards, > Zoran > > + > return rc; > } > > diff --git a/osaf/libs/saf/libSaNtf/Makefile.am > b/osaf/libs/saf/libSaNtf/Makefile.am > --- a/osaf/libs/saf/libSaNtf/Makefile.am > +++ b/osaf/libs/saf/libSaNtf/Makefile.am > @@ -29,6 +29,7 @@ lib_LTLIBRARIES = libSaNtf.la > libSaNtf_la_SOURCES = > > libSaNtf_la_CPPFLAGS = \ > + -DSA_EXTENDED_NAME_SOURCE \ > $(AM_CPPFLAGS) > > if HAVE_LD_VERSION_SCRIPT > > ------------------------------------------------------------------------------ > 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
