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
