Sorry. Please read it as "I would like to push it today"
Thanks,
Praveen
On 29-Aug-16 2:25 PM, praveen malviya wrote:
> Hi Gary,
>
> Any update on this.
> I think all non-major comments can be taken post FC also.
> If you do not any major comments, I would not like to push it today.
>
> Thanks,
> Praveen
>
> On 26-Aug-16 6:40 PM, praveen malviya wrote:
>> 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
>>
>
> ------------------------------------------------------------------------------
> _______________________________________________
> Opensaf-devel mailing list
> Opensaf-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/opensaf-devel
>
------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
