Hi Gary,
Please see inline with [Praveen].
Thanks,
Praveen
On 26-Aug-16 6:25 PM, Gary Lee wrote:
> Hi Praveen
>
>
> On 26/08/2016 10:19 PM, praveen malviya wrote:
>> Hi Gary,
>>
>> Thanks for the comments.
>> Please see responses inline with [Praveen]
>>
>>
>> Thanks,
>> Praveen
>>
>>
>>> +// GL: very similar version in d2nmsg.c. Also this isn't freeing the
>>> SaNameTs attrs.list
>> [Praveen] I did not get it completely.
>> The one in utils.cc is used by AMFD to free the message inside
>> avd_d2n_msg_dequeue() and other one that is present in d2nmsg.c is
>> used by AMFND to free the message.
>> In utils.cc: delete [] (compcsi->info.attrs.list) is already present.
>> In d2nmsg.c: AMFND will have to free memory allocated for extended
>> names names (if any) by leap.
>
> I mean since 'list' is of type AVSV_ATTR_NAME_VAL, the SaNameTs inside
> AVSV_ATTR_NAME_VAL are not freed.
[Praveen] From utils.cc perpective it is connected with the below
expalnation of memcpy(). Since we are using memcpy() we are not freeing
in utils.cc for SaNameT for both SU_SI_ASSIGN message and this message.
Since amfnd uses d2nmsg.cm it will free because in decoding there is
allocation of memory by leap decoding APIs for SaNameT .
I will do change for both SUSI assign and this new message.
But in that case we do not require a separate function in utils.cc. We
can directly use d2nmsg.cas it is generic. Do you agree?
Also as of now these is not harming because we are not corrupting memory
inside CSI->list_attributes even if there is any long dn configured.I
will try to fix this on monday. otherwise post FC. Do you agree?
Thanks,
Praveen
>>> static void free_d2n_compcsi_info(AVSV_DND_MSG *compcsi_msg) {
>>> AVSV_D2N_COMPCSI_ASSIGN_MSG_INFO *compcsi =
>>> &compcsi_msg->msg_info.d2n_compcsi_assign_msg_info;
>>>
>>> @@ -2030,6 +2031,7 @@
>>> /* Scan the list of attributes for the CSI and add it to the
>>> message */
>>> while ((attr_ptr_db != nullptr) &&
>>> (ptr_csiattr_msg->number <
>>> compcsi->csi->num_attributes)) {
>>> + // GL: AVSV_ATTR_NAME_VAL contains SaNameT. Does it need to
>>> be deep copied.
>> [Praveen] I got the same doubt while reviewing long dn patches because
>> same mechamism is being done in avd_prep_csi_attr_info() for existing
>> SUSI assign message. I thought it works because in case of long dn it
>> will copy reference to the longdn and in case of short dn it will copy
>> the original string. Since it is not freed in free_d2n_*() functions
>> original referenced string in CSI is never gets corrupted.
>> What do you think?
>
> Good point. We should fix avd_prep_csi_attr_info() too? I think there is
> a danger the original SaNameT will no longer be available, since these
> messages go into a queue that gets transmitted later?
>
> Thanks
> Gary
>
------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
