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
