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
