Hi Praveen,
The case you just mentioned is still in callback context, so Agent can
help application to release the allocated notification. But still
another case:
+ SaAmfProtectionGroupNotificationBufferT buff;
+ buff.notification = NULL;
+ rc = saAmfProtectionGroupTrack_4(my_amf_hdl, &track_csi,
SA_TRACK_CURRENT, &buff);
+ if (rc != SA_AIS_OK) {
+ syslog(LOG_ERR, "saAmfProtectionGroupTrack FAILED - %u", rc);
+ goto done;
+ }
In this case Agent has to allocate notification but it's not in Agent's
context.
Application has to call API Free_4(buff.notification) to free up
notification.
In order to iterate to free longDn(s) inside Free_4(), Agent has to
memorize a list numberOfItems for every single call as above Track_4(),
or Agent can add sentinel element to the allocated notification.
Thanks,
Minh
On 22/08/16 15:34, praveen malviya wrote:
> Hi,
> The callback looks like this:
> typedef void
> (*SaAmfProtectionGroupTrackCallbackT_4)(
> const SaNameT *csiName,
> SaAmfProtectionGroupNotificationBufferT_4 *notificationBuffer,
> SaUint32T numberOfMembers,
> SaAisErrorT error);
>
> Inside this callback, application is supposed to call
> saAmfProtectionGroupNotificationFree_4(). So agent must be able to
> deduce this information as SaAmfProtectionGroupNotificationBufferT_4
> contains numberOfItems and also numberOfMembers is available from
> callback.
> Since B.04.01 APIs are not fully implemented, agent copies from old
> type of structure to new type in ava_cpy_protection_group_ntf().
>
>
> Thanks,
> Praveen
>
> On 22-Aug-16 10:51 AM, minh chau wrote:
>> Hi Praveen,
>>
>> The problem with B.04.01 is the API:
>> saAmfProtectionGroupNotificationFree_4(SaAmfHandleT hdl,
>> SaAmfProtectionGroupNotificationT_4 *notification) does not have
>> numberOfItems.
>> Agent does not know how many element in *notification, each of element
>> can hide a longDn inside it.
>>
>> Thanks,
>> Minh
>>
>>
>> On 22/08/16 15:04, praveen malviya wrote:
>>> Hi Minh,
>>>
>>> SaAmfProtectionGroupNotificationBufferT_4() contains numberOfItems to
>>> iterate over. In case of B.04.01, it should be simple as agent can
>>> call direclty osaf_extended_name_free() during iteration inside
>>> saAmfProtectionGroupNotificationFree_4(). So I think, only a for loop
>>> which will iterate over numberOfItems is required.
>>>
>>> Problem was in B.01.01 case, where application will have to iterate
>>> and free the memory. For this, Long has already suggested and that
>>> needs to be documented.
>>>
>>>
>>> Thanks,
>>> Praveen
>>>
>>>
>>> On 20-Aug-16 2:22 PM, minh chau wrote:
>>>> Hi Long, Praveen,
>>>>
>>>> Regarding this TODO
>>>> + if(notification) {
>>>> + // TODO (minhchau): memleak if notification is an array
>>>> + osaf_extended_name_free(¬ification->member.compName);
>>>> free(notification);
>>>> + }
>>>>
>>>> Client currently uses saAmfProtectionGroupNotificationFree_4(handle,
>>>> buff->notification) to free the notification in buffer.
>>>> If @buff->notification is a list of shortDn only, that should work as
>>>> before, as agent will call this inside
>>>> saAmfProtectionGroupNotificationFree_4
>>>>
>>>> /* free memory */
>>>> if(notification)
>>>> free(notification);
>>>>
>>>> It will cause memory leak if @buff->notification contains a list of
>>>> longDN notifications.
>>>> The leak is longDn of compName in each notification after the the
>>>> first
>>>> one in the array @buff->notification.
>>>>
>>>> Agent can add a sentinel element when agent allocates
>>>> @buff->notification, set this last element as NULL
>>>> In Free() API, agent could iterate and free longDn in each element of
>>>> array @buff->notification until agent reaches NULL element.
>>>>
>>>> Do you think it could work?
>>>
>>>>
>>>> Thanks,
>>>> Minh
>>>>
>>>> On 19/08/16 21:13, Long Nguyen wrote:
>>>>> Hi Praveen,
>>>>>
>>>>> Please see my answers marked with [Long].
>>>>>
>>>>> Best regards,
>>>>> Long Nguyen.
>>>>>
>>>>> On 8/19/2016 6:01 PM, praveen malviya wrote:
>>>>>> Hi Long,
>>>>>>
>>>>>> I see one problem if B.01.01 application frees the memory in pg
>>>>>> tracking callback.
>>>>>> Please see inline.
>>>>>>
>>>>>> Thanks,
>>>>>> Praveen
>>>>>> On 19-Aug-16 12:00 PM, Long HB Nguyen wrote:
>>>>>>> osaf/libs/agents/saf/amfa/amf_agent.cc | 1 +
>>>>>>> osaf/libs/agents/saf/amfa/ava_hdl.cc | 2 --
>>>>>>> 2 files changed, 1 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>>
>>>>>>> diff --git a/osaf/libs/agents/saf/amfa/amf_agent.cc
>>>>>>> b/osaf/libs/agents/saf/amfa/amf_agent.cc
>>>>>>> --- a/osaf/libs/agents/saf/amfa/amf_agent.cc
>>>>>>> +++ b/osaf/libs/agents/saf/amfa/amf_agent.cc
>>>>>>> @@ -2450,6 +2450,7 @@ SaAisErrorT AmfAgent::ProtectionGroupTra
>>>>>>> ava_cpy_protection_group_ntf(buf->notification,
>>>>>>> rsp_buf->notification,
>>>>>>> buf->numberOfItems,
>>>>>>> SA_AMF_HARS_READY_FOR_ASSIGNMENT);
>>>>>>> rc = SA_AIS_ERR_NO_SPACE;
>>>>>>> + buf->numberOfItems = rsp_buf->numberOfItems;
>>>>>>> }
>>>>>>> } else { /* if(create_memory == false) */
>>>>>>>
>>>>>>> diff --git a/osaf/libs/agents/saf/amfa/ava_hdl.cc
>>>>>>> b/osaf/libs/agents/saf/amfa/ava_hdl.cc
>>>>>>> --- a/osaf/libs/agents/saf/amfa/ava_hdl.cc
>>>>>>> +++ b/osaf/libs/agents/saf/amfa/ava_hdl.cc
>>>>>>> @@ -697,7 +697,6 @@ uint32_t ava_hdl_cbk_rec_prc(AVSV_AMF_CB
>>>>>>> ((SaAmfCallbacksT_4*)reg_cbk)->saAmfProtectionGroupTrackCallback(&pg_track->csi_name,
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> &buf,
>>>>>>> pg_track->mem_num, pg_track->err);
>>>>>>> - free(buf.notification);
>>>>>>> } else {
>>>>>>> pg_track->err = SA_AIS_ERR_NO_MEMORY;
>>>>>>> LOG_CR("Notification is NULL: Invoking
>>>>>>> PGTrack Callback with error SA_AIS_ERR_NO_MEMORY");
>>>>>>> @@ -740,7 +739,6 @@ uint32_t ava_hdl_cbk_rec_prc(AVSV_AMF_CB
>>>>>>> ((SaAmfCallbacksT
>>>>>>> *)reg_cbk)->saAmfProtectionGroupTrackCallback(&pg_track->csi_name,
>>>>>>> &buf,
>>>>>>> pg_track->mem_num, pg_track->err);
>>>>>>> - free(buf.notification);
>>>>>> For B.04.01 API, saAmfProtectionGroupNotificationFree_4() is taking
>>>>>> care of freeing any extended name. For
>>>>>> saAmfProtectionGroupNotificationFree(), it is the application's
>>>>>> responsibility to free the memory. But how it will free any extended
>>>>>> name.
>>>>>> I think there is no API equivalent to osaf_extended_name_free() for
>>>>>> application. Is there any way?
>>>>>> [Long] I think we can do somethings like in applications:
>>>>>> if (strlen(saAisNameBorrow(buff.notification[i].member.comp_name)) >
>>>>>> SA_MAX_UNEXTENDED_NAME_LENGTH)
>>>>>> free(saAisNameBorrow(buff.notification[i].member.comp_name));
>>>>>>
>>>>>> Otherwise we can document it that:
>>>>>> -if compName is not long dn in the notification buffer, then
>>>>>> application has to free the memory.This will provide backward
>>>>>> compatibility and spec compliance.
>>>>>>
>>>>>> -if compName is longdn in notification then application should not
>>>>>> free the memory. Agent will free the memory after callback is
>>>>>> completed. So any B.01.01 application adapting to long dn will take
>>>>>> care of this when modifying the application. In that case we need to
>>>>>> do something like this :
>>>>>> for i in notificationBuffer->notification[i]
>>>>>> if
>>>>>> (osaf_is_an_extended_name(notificationBuffer->notification[i].member.compName)){
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> longdn_found = true;
>>>>>> osaf_extended_name_free();
>>>>>> }
>>>>>> }
>>>>>> if(longdn_found)
>>>>>> free(buf.notification)
>>>>>>
>>>>>>
>>>>>> Also there is a Todo in amf_agent.cc " // TODO (minhchau):
>>>>>> memleak if
>>>>>> notification is an array".
>>>>>> [Long] Thanks, I will check it.
>>>>>>
>>>>>> Thanks,
>>>>>> Praveen
>>>>>>
>>>>>>> } else {
>>>>>>> pg_track->err = SA_AIS_ERR_NO_MEMORY;
>>>>>>> LOG_CR("Notification is NULL: Invoking
>>>>>>> PGTrack Callback with error SA_AIS_ERR_NO_MEMORY");
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>
------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
