Hi Gary, Thanks for quick review. I will change it to pointer.
But I think functionality perspective both reference and pointer will work. For APIS calls there will be implicit conversion, I think. But cppcheck may catch it the API calls. Anyways I will push it. Thanks, Praveen On 22-Nov-16 7:13 AM, Gary Lee wrote: > Hi Praveen > > Sorry, I should have said change the handle to a pointer, not reference. > > Ack (review) with comments below: > >> On 21 Nov. 2016, at 6:13 pm, [email protected] >> <mailto:[email protected]> wrote: >> >> osaf/services/saf/amf/amfd/ntf.cc <http://ntf.cc> | 17 ++++++++++++++--- >> 1 files changed, 14 insertions(+), 3 deletions(-) >> >> >> Notifications are pused in job queue (fixed in ticket #314). >> During switchover, active controller will empty the job queue. >> Since notifications allocate memory it needs to be freed. >> >> diff --git a/osaf/services/saf/amf/amfd/ntf.cc <http://ntf.cc> >> b/osaf/services/saf/amf/amfd/ntf.cc <http://ntf.cc> >> --- a/osaf/services/saf/amf/amfd/ntf.cc <http://ntf.cc> >> +++ b/osaf/services/saf/amf/amfd/ntf.cc <http://ntf.cc> >> @@ -546,14 +546,12 @@ SaAisErrorT avd_try_send_notification(Nt >> SaNtfNotificationsT *myntf = &job->myntf; >> SaAisErrorT rc = SA_AIS_OK; >> SaNtfNotificationHeaderT *header = nullptr; >> - SaNtfNotificationHandleT notificationHandle = 0; >> + SaNtfNotificationHandleT ¬ificationHandle = >> myntf->notification.stateChangeNotification.notificationHandle; > > [GL] Please change to SaNtfNotificationHandleT* notificationHandle = > nullptr; > >> >> if (myntf->notificationType == SA_NTF_TYPE_STATE_CHANGE) { >> header = >> &myntf->notification.stateChangeNotification.notificationHeader; >> - notificationHandle = >> myntf->notification.stateChangeNotification.notificationHandle; > > [GL] Please restore above line with notificationHandle = &myntf etc > >> } else if (myntf->notificationType == SA_NTF_TYPE_ALARM) { >> header = &myntf->notification.alarmNotification.notificationHeader; >> - notificationHandle = >> myntf->notification.alarmNotification.notificationHandle; > > [GL] Please restore above line with notificationHandle = &myntf etc > > } >> >> //Try to send the notification if not sent. >> @@ -573,6 +571,8 @@ SaAisErrorT avd_try_send_notification(Nt >> rc = saNtfNotificationFree(notificationHandle); >> if ((rc == SA_AIS_ERR_TRY_AGAIN) || (rc == SA_AIS_ERR_TIMEOUT)) { >> TRACE("Notification Free unsuccesful TRY_AGAIN or TIMEOUT rc:%u", rc); >> + } else { >> + notificationHandle = 0; >> } >> >> done: >> @@ -887,4 +887,15 @@ AvdJobDequeueResultT NtfSend::exec(const >> } >> >> NtfSend::~NtfSend() { >> + SaAisErrorT rc = SA_AIS_OK; >> + if (myntf.notificationType == SA_NTF_TYPE_STATE_CHANGE) { >> + if (myntf.notification.stateChangeNotification.notificationHandle >> != 0) >> + rc = >> saNtfNotificationFree(myntf.notification.stateChangeNotification.notificationHandle); >> + } else if (myntf.notificationType == SA_NTF_TYPE_ALARM) { >> + if (myntf.notification.alarmNotification.notificationHandle != 0) >> + rc = >> saNtfNotificationFree(myntf.notification.alarmNotification.notificationHandle); >> + } >> + if (rc != SA_AIS_OK) { >> + TRACE("Notification Free failed rc:%u", rc); >> + } >> } > ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
