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
