Thanks Lennart. I have added comments to make the code more clear/understandable.
Regards, Vu > -----Original Message----- > From: Lennart Lund [mailto:lennart.l...@ericsson.com] > Sent: Monday, August 22, 2016 6:14 PM > To: Vu Minh Nguyen <vu.m.ngu...@dektech.com.au>; Minh Hon Chau > <minh.c...@dektech.com.au> > Cc: opensaf-devel@lists.sourceforge.net > Subject: RE: [devel] [PATCH 1 of 1] ntfsv: refactor logging long dn > notification [#1585] > > Hi Vu > > Ack with comments: > It seems as if you have integrated the manual steps by adding some functions > for handling data in IMM but it's very cryptic. > It's hard to see what "environment" that is backed up, restored and setup. > This should be made more clear. Since the functions used for this are not > generically handling any environment the names could be changed to reflect > what they actually do, also variables used could have more descriptive > names instead of aData, CData etc. Also a comment summarizing what's > going on would not be out of place... > > Thanks > Lennart > > > -----Original Message----- > > From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au] > > Sent: den 22 augusti 2016 10:49 > > To: Lennart Lund <lennart.l...@ericsson.com>; Minh Hon Chau > > <minh.c...@dektech.com.au> > > Subject: RE: [devel] [PATCH 1 of 1] ntfsv: refactor logging long dn > notification > > [#1585] > > > > Hi Lennart, Minh > > > > Here is the patch just in case. > > > > If you have no comment, I will push it today. Thanks. :) > > > > Regards, Vu > > > > > -----Original Message----- > > > From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au] > > > Sent: Tuesday, August 16, 2016 10:30 AM > > > To: 'praveen malviya' <praveen.malv...@oracle.com>; 'Lennart Lund' > > > <lennart.l...@ericsson.com>; 'Minh Hon Chau' > > > <minh.c...@dektech.com.au> > > > Cc: 'opensaf-devel@lists.sourceforge.net' <opensaf- > > > de...@lists.sourceforge.net> > > > Subject: RE: [devel] [PATCH 1 of 1] ntfsv: refactor logging long dn > > > notification [#1585] > > > > > > Hi all, > > > > > > Do you have any comments on the updated patch (update test code)? > > > Thanks. > > > > > > Regards, Vu > > > > > > > -----Original Message----- > > > > From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au] > > > > Sent: Thursday, August 11, 2016 3:33 PM > > > > To: praveen malviya <praveen.malv...@oracle.com>; Lennart Lund > > > > <lennart.l...@ericsson.com>; Minh Hon Chau > > > > <minh.c...@dektech.com.au> > > > > Cc: opensaf-devel@lists.sourceforge.net > > > > Subject: [devel] [PATCH 1 of 1] ntfsv: refactor logging long dn > > notification > > > > [#1585] > > > > > > > > osaf/services/saf/ntfsv/ntfs/NtfLogger.cc | 51 +----- > > > > tests/ntfsv/tet_longDnObject_notification.c | 188 > > > > +++++++++++++++++++++++++++- > > > > 2 files changed, 196 insertions(+), 43 deletions(-) > > > > > > > > > > > > Remove the part of code that truncates the long DN. > > > > And update the long DN test suite (#36) to make sure full record logged. > > > > > > > > diff --git a/osaf/services/saf/ntfsv/ntfs/NtfLogger.cc > > > > b/osaf/services/saf/ntfsv/ntfs/NtfLogger.cc > > > > --- a/osaf/services/saf/ntfsv/ntfs/NtfLogger.cc > > > > +++ b/osaf/services/saf/ntfsv/ntfs/NtfLogger.cc > > > > @@ -21,6 +21,7 @@ > > > > */ > > > > #include <sys/poll.h> > > > > > > > > +#include "osaf_utility.h" > > > > #include "saAis.h" > > > > #include "saLog.h" > > > > #include "NtfAdmin.hh" > > > > @@ -232,48 +233,22 @@ SaAisErrorT NtfLogger::logNotification(N > > > > notif->getNotificationId(), > > > > SA_LOG_RECORD_WRITE_ACK, > > > > &logRecord); > > > > - if (SA_AIS_OK != errorCode) { > > > > - LOG_NO("Failed to log an alarm or security alarm notification > > (%d)", > > > > errorCode); > > > > - if (errorCode == SA_AIS_ERR_LIBRARY || errorCode == > > > > SA_AIS_ERR_BAD_HANDLE) { > > > > - LOG_ER("Fatal error SA_AIS_ERR_LIBRARY or > > > > SA_AIS_ERR_BAD_HANDLE; exiting (%d)...", errorCode); > > > > - exit(EXIT_FAILURE); > > > > - } else if (errorCode == SA_AIS_ERR_INVALID_PARAM) { > > > > - /* Retry to log truncated notificationObject/notifyingObject > > because > > > > - * LOG Service has not supported long dn in Opensaf 4.5 > > > > - */ > > > > - char short_dn[SA_MAX_UNEXTENDED_NAME_LENGTH]; > > > > - memset(&short_dn, 0, SA_MAX_UNEXTENDED_NAME_LENGTH); > > > > - SaNameT shortdn_notificationObject, shortdn_notifyingObject; > > > > - if (osaf_is_an_extended_name(ntfHeader->notificationObject)) { > > > > - strncpy(short_dn, osaf_extended_name_borrow(ntfHeader- > > > > >notificationObject) > > > > - , SA_MAX_UNEXTENDED_NAME_LENGTH - 1); > > > > - osaf_extended_name_lend(short_dn, > > &shortdn_notificationObject); > > > > - logRecord.logHeader.ntfHdr.notificationObject = > > > > &shortdn_notificationObject; > > > > - } > > > > - if (osaf_is_an_extended_name(ntfHeader->notifyingObject)) { > > > > - strncpy(short_dn, osaf_extended_name_borrow(ntfHeader- > > > > >notifyingObject) > > > > - , SA_MAX_UNEXTENDED_NAME_LENGTH - 1); > > > > - osaf_extended_name_lend(short_dn, &shortdn_notifyingObject); > > > > - logRecord.logHeader.ntfHdr.notifyingObject = > > > > &shortdn_notifyingObject; > > > > - } > > > > - if (short_dn[0] != '\0') { > > > > - LOG_NO("Retry to log the truncated > > > > notificationObject/notifyingObject"); > > > > - if ((errorCode = saLogWriteLogAsync(alarmStreamHandle, > > > > - > > notif->getNotificationId(), > > > > - SA_LOG_RECORD_WRITE_ACK, > > > > - &logRecord)) != > > SA_AIS_OK) { > > > > - LOG_ER("Failed to log the truncated > > > > notificationObject/notifyingObject (%d)" > > > > - , errorCode); > > > > - } > > > > - } > > > > - } > > > > - goto end; > > > > + switch (errorCode) { > > > > + case SA_AIS_OK: > > > > + break; > > > > + > > > > + /* LOGsv is busy. Put the notification to queue and re-send next > > time */ > > > > + case SA_AIS_ERR_TRY_AGAIN: > > > > + case SA_AIS_ERR_TIMEOUT: > > > > + TRACE("Failed to log notification (ret: %d). Try next time.", > > > > errorCode); > > > > + break; > > > > + > > > > + default: > > > > + osaf_abort(errorCode); > > > > } > > > > } > > > > > > > > -end: > > > > TRACE_LEAVE(); > > > > - > > > > return errorCode; > > > > } > > > > > > > > diff --git a/tests/ntfsv/tet_longDnObject_notification.c > > > > b/tests/ntfsv/tet_longDnObject_notification.c > > > > --- a/tests/ntfsv/tet_longDnObject_notification.c > > > > +++ b/tests/ntfsv/tet_longDnObject_notification.c > > > > @@ -19,6 +19,7 @@ > > > > */ > > > > #include <utest.h> > > > > #include <util.h> > > > > +#include <unistd.h> > > > > #include "tet_ntf.h" > > > > #include "tet_ntf_common.h" > > > > //#include "util.h" > > > > @@ -57,6 +58,166 @@ static SaNtfSecurityAlarmNotificationT m > > > > extern void saAisNameLend(SaConstStringT value, SaNameT* name); > > > > extern SaConstStringT saAisNameBorrow(const SaNameT* name); > > > > > > > > +//> > > > > +// For backup and restore IMM attribute values. > > > > +//< > > > > + > > > > +#define MAX_DATA 256 > > > > +typedef struct { > > > > + char name[MAX_DATA]; > > > > + char val[MAX_DATA]; > > > > + int val_is_num; > > > > +} attrinfo_t; > > > > + > > > > +typedef struct { > > > > + attrinfo_t *attr; > > > > + size_t size; > > > > +} attrlist_t; > > > > + > > > > +typedef struct { > > > > + attrlist_t *alist; > > > > + char dn[MAX_DATA]; > > > > +} imminfo_t; > > > > + > > > > +static void getVal(imminfo_t *info) > > > > +{ > > > > + FILE *fp = NULL; > > > > + attrinfo_t *tmp = NULL; > > > > + char attrValue[MAX_DATA] = {0}; > > > > + char command[MAX_DATA] = {0}; > > > > + size_t s = info->alist->size; > > > > + > > > > + tmp = info->alist->attr; > > > > + while (s) { > > > > + sprintf(command, "immlist -a %s %s " > > > > + "| awk -F \"=\" '{print $2}' ", tmp->name, > > > > + info->dn); > > > > + fp = popen(command, "r"); > > > > + while (fgets(attrValue, sizeof(attrValue) - 1, fp) != NULL) > > {}; > > > > + pclose(fp); > > > > + strtok(attrValue, "\n"); > > > > + strncpy(tmp->val, attrValue, MAX_DATA); > > > > + s--; > > > > + tmp++; > > > > + > > > > + /* For Debug only*/ > > > > + if (getenv("NTFTEST_DEBUG")) printf("%s \n", command); > > > > + > > > > + } > > > > + > > > > +} > > > > + > > > > +static int setVal(imminfo_t *info) > > > > +{ > > > > + char *f = NULL; > > > > + attrinfo_t *tmp = NULL; > > > > + char command[MAX_DATA] = {0}; > > > > + char format[MAX_DATA] = {0}; > > > > + size_t s = info->alist->size; > > > > + int rc; > > > > + > > > > + f = format; > > > > + tmp = info->alist->attr; > > > > + while (s) { > > > > + if (tmp->val_is_num) { > > > > + rc = snprintf(f, MAX_DATA, "-a %s=%s ", > > > > + tmp->name, > > > > + tmp->val); > > > > + } else { > > > > + rc = snprintf(f, MAX_DATA, "-a %s='%s' ", > > > > + tmp->name, > > > > + tmp->val); > > > > + } > > > > + f = f + rc; > > > > + s--; > > > > + tmp++; > > > > + } > > > > + sprintf(command, "immcfg %s %s", format, info->dn); > > > > + > > > > + /* For Debug only*/ > > > > + if (getenv("NTFTEST_DEBUG")) printf("%s \n", command); > > > > + > > > > + rc = system(command); > > > > + return WEXITSTATUS(rc); > > > > +} > > > > + > > > > +static attrinfo_t g_alarm[] = { > > > > + { > > > > + "saLogStreamFixedLogRecordSize", > > > > + "200", // default val > > > > + 1 // val is num type > > > > + }, > > > > + { > > > > + "saLogStreamLogFileFormat", > > > > + "@Cr @Ct @Nt @Ne6 @No30 @Ng30 \"@Cb\"", > > > > + 0 // val is string type > > > > + } > > > > +}; > > > > +static attrinfo_t g_config[] = { > > > > + { > > > > + "logMaxLogrecsize", > > > > + "1024", > > > > + 1 > > > > + } > > > > +}; > > > > +static attrlist_t alarmList = {g_alarm, > > sizeof(g_alarm)/sizeof(attrinfo_t)}; > > > > +static attrlist_t configList = {g_config, > > sizeof(g_config)/sizeof(attrinfo_t)}; > > > > +static imminfo_t alarmInfo = {&alarmList, > > > > "safLgStrCfg=saLogAlarm,safApp=safLogService"}; > > > > +static imminfo_t configInfo = {&configList, > > > > "logConfig=1,safApp=safLogService"}; > > > > + > > > > +static void backupEnv(void) > > > > +{ > > > > + getVal(&alarmInfo); > > > > + getVal(&configInfo); > > > > + > > > > + /* For Debug only*/ > > > > + if (getenv("NTFTEST_DEBUG")) printf("\n"); > > > > +} > > > > + > > > > +static void restoreEnv(void) > > > > +{ > > > > + /* For Debug only*/ > > > > + if (getenv("NTFTEST_DEBUG")) printf("\n"); > > > > + > > > > + setVal(&configInfo); > > > > + setVal(&alarmInfo); > > > > + > > > > + /* For Debug only*/ > > > > + if (getenv("NTFTEST_DEBUG")) printf("\n"); > > > > +} > > > > + > > > > +static void setupEnv(void) > > > > +{ > > > > + > > > > + attrinfo_t aData[2]; > > > > + memcpy(aData, g_alarm, sizeof(g_alarm)); > > > > + strcpy(aData[0].val, "0"); > > > > + strcpy(aData[1].val, "@Cr @Ct @Nt @Ne6 @No @Ng \"@Cb\""); > > > > + > > > > + attrinfo_t cData[1]; > > > > + memcpy(cData, g_config, sizeof(g_config)); > > > > + strcpy(cData[0].val, "65535"); > > > > + > > > > + attrlist_t aList = {aData, sizeof(aData)/sizeof(attrinfo_t)}; > > > > + attrlist_t cList = {cData, sizeof(cData)/sizeof(attrinfo_t)}; > > > > + > > > > + imminfo_t aInfo; > > > > + aInfo.alist= &aList; > > > > + strcpy(aInfo.dn, alarmInfo.dn); > > > > + > > > > + imminfo_t cInfo; > > > > + cInfo.alist= &cList; > > > > + strcpy(cInfo.dn, configInfo.dn); > > > > + > > > > + setVal(&aInfo); > > > > + setVal(&cInfo); > > > > + > > > > + /* For Debug only*/ > > > > + if (getenv("NTFTEST_DEBUG")) printf("\n"); > > > > +} > > > > + > > > > +//< > > > > + > > > > /** > > > > * Init default long dn objects > > > > */ > > > > @@ -306,6 +467,7 @@ void extFillHeader(SaNtfNotificationHead > > > > int i; > > > > SaStringT dest_ptr; > > > > SaNameT name1, name2, name3, name4, name5; > > > > + > > > > *(head->eventType) = SA_NTF_ALARM_COMMUNICATION; > > > > *(head->eventTime) = SA_TIME_UNKNOWN; > > > > > > > > @@ -390,12 +552,15 @@ void extFillHeader(SaNtfNotificationHead > > > > */ > > > > void extAdditionalInfoTest(void) > > > > { > > > > - SaNtfAlarmNotificationFilterT myAlarmFilter; > > > > + SaNtfAlarmNotificationFilterT myAlarmFilter; > > > > subscriptionId = 1; > > > > SaNtfNotificationHeaderT *head; > > > > - > > > > + > > > > rc = SA_AIS_OK; > > > > > > > > + backupEnv(); > > > > + setupEnv(); > > > > + > > > > resetCounters(); > > > > > > > > safassert(saNtfInitialize(&ntfHandle, &ntfCbTest, &ntfVersion) , > > > > SA_AIS_OK); > > > > @@ -475,6 +640,7 @@ void extFillHeader(SaNtfNotificationHead > > > > safassert(saNtfFinalize(ntfHandle), SA_AIS_OK); > > > > rc = check_errors(); > > > > test_validate(rc, SA_AIS_OK); > > > > + restoreEnv(); > > > > } > > > > > > > > /** > > > > @@ -482,12 +648,15 @@ void extFillHeader(SaNtfNotificationHead > > > > */ > > > > void extFilterNotificationTest(void) > > > > { > > > > - SaNtfAlarmNotificationFilterT myAlarmFilter; > > > > + SaNtfAlarmNotificationFilterT myAlarmFilter; > > > > subscriptionId = 1; > > > > SaNtfNotificationHeaderT *head; > > > > - > > > > + > > > > rc = SA_AIS_OK; > > > > > > > > + backupEnv(); > > > > + setupEnv(); > > > > + > > > > resetCounters(); > > > > > > > > safassert(saNtfInitialize(&ntfHandle, &ntfCbTest, &ntfVersion) , > > > > SA_AIS_OK); > > > > @@ -586,6 +755,7 @@ void extFilterNotificationTest(void) > > > > safassert(saNtfFinalize(ntfHandle), SA_AIS_OK); > > > > rc = check_errors(); > > > > test_validate(rc, SA_AIS_OK); > > > > + restoreEnv(); > > > > } > > > > > > > > /** > > > > @@ -593,11 +763,14 @@ void extFilterNotificationTest(void) > > > > */ > > > > void extAlarmNotificationTest(void) > > > > { > > > > - SaNtfAlarmNotificationFilterT myAlarmFilter; > > > > + SaNtfAlarmNotificationFilterT myAlarmFilter; > > > > subscriptionId = 1; > > > > > > > > rc = SA_AIS_OK; > > > > > > > > + backupEnv(); > > > > + setupEnv(); > > > > + > > > > resetCounters(); > > > > > > > > safassert(saNtfInitialize(&ntfHandle, &ntfCbTest, &ntfVersion) , > > > > SA_AIS_OK); > > > > @@ -683,6 +856,7 @@ void extAlarmNotificationTest(void) > > > > safassert(saNtfFinalize(ntfHandle), SA_AIS_OK); > > > > rc = check_errors(); > > > > test_validate(rc, SA_AIS_OK); > > > > + restoreEnv(); > > > > } > > > > > > > > /** > > > > @@ -887,6 +1061,9 @@ void extSecurityAlarmNotificationTest(vo > > > > > > > > subscriptionId = 5; > > > > > > > > + backupEnv(); > > > > + setupEnv(); > > > > + > > > > resetCounters(); > > > > safassert(saNtfInitialize(&ntfHandle, &ntfCbTest, &ntfVersion) , > > > > SA_AIS_OK); > > > > safassert(saNtfSelectionObjectGet(ntfHandle, &selectionObject) , > > > > SA_AIS_OK); > > > > @@ -954,6 +1131,7 @@ void extSecurityAlarmNotificationTest(vo > > > > safassert(saNtfFinalize(ntfHandle) , SA_AIS_OK); > > > > rc = check_errors(); > > > > test_validate(rc, SA_AIS_OK); > > > > + restoreEnv(); > > > > } > > > > > > > > __attribute__ ((constructor)) static void > > > > longDnObject_notification_constructor(void) > > > > > > > > > > ---------------------------------------------------------------------------- > > -- > > > > What NetFlow Analyzer can do for you? Monitors network bandwidth > > and > > > > traffic > > > > patterns at an interface-level. Reveals which users, apps, and protocols > > are > > > > consuming the most bandwidth. Provides multi-vendor support for > > > NetFlow, > > > > J-Flow, sFlow and other flows. Make informed decisions using capacity > > > > planning reports. http://sdm.link/zohodev2dev > > > > _______________________________________________ > > > > Opensaf-devel mailing list > > > > Opensaf-devel@lists.sourceforge.net > > > > https://lists.sourceforge.net/lists/listinfo/opensaf-devel ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
