Hi Praveen,
Agree this discussion should be continued, currently it causes a leak.
Thanks,
Minh
On 23/08/16 16:57, praveen malviya wrote:
> Hi Minh,
>
> I am going through the agent code to see if something can be done. I
> think, since B.04.01 APIs are not implemented this problem is coming.
> But still all longdn patches can be pushed and this discussion can
> continue.
>
> What do you think?
>
>
> Thanks,
> praveen
>
>
>
> On 22-Aug-16 12:08 PM, minh chau wrote:
>> 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
