Hi Minh,
I think this is subject to interpretation. After finalize(), handle
becomes invalid. So an application cannot call Free_4() to free any
memory. In such a case, freeing any resources associated with this
handle in finalize() seems to be ok.
Anyways freeing in finalize() can be postponed for any real use case
to come up.In that case please go ahead by adding sentinel element.
I think there are not other pending things in #1642 other than this.
Thanks,
Praveen
On 26-Aug-16 7:35 AM, minh chau wrote:
> Hi Praveen,
>
> Just to confirm if I understand correctly the problem you mentioned in
> saAmfFinalize(). As the specification says application should call
> free()/Free_4() to release the allocated memory, if application does not
> release memory then it's most likely application misuses API. Or do you
> mean saAmfFinalize() should do the same as saNtfFinalize()? if this is
> the case it looks just an enhancement to guard AMFA?
>
> Thanks
> Minh
>
> On 25/08/16 20:04, praveen malviya wrote:
>> Hi Minh,
>>
>> AMFA currently does not remember the allocated memory. It relies on
>> the application always to free the memory. In saAmfFinalize() also, it
>> does not free the memory. I think AMFA should remember memory by
>> associating it with handle because process which is starting PG
>> tracking may not be a component and may not call free()/Free_4() and
>> just relies on saAmfFinalize() call to release all the resources.
>>
>> I think as of now please go ahead with your suggested solution. From
>> finalize perspective this is a defect and applicable to all the
>> branches. So please raise a ticket for that.
>>
>> 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
