I assume the side effect that modifies the variable i_su->list_of_susi 
in the loop conditional happens at the following lines in the code you sent:

     /* now delete it from the SU list */
     if (p_su_si == NULL) {
         susi->su->list_of_susi = susi->su_next;

This only happens when p_su_si is NULL. What happens if p_su_si is not 
NULL? Will i_su->list_of_susi have the same value also in the next 
iteration? free(susi) is executed unconditionally at the end of the 
avd_susi_delete() function, though there are a couple of return 
statements in some branches in the code above it.

regards,
Anders Widell

The code is
2013-07-29 14:14, Anders Widell skrev:
> Thanks for your analysis. I still don't understand the code, but if you
> think this warning is a false positive I take your word for it. Some
> additional info from the warning:
>
> The free happens at line 504 in file avd_siass.c:
>       free(susi);
>
> The dereference happens at line 1070 in file avd_csi.c:
>       while (susi->list_of_csicomp != NULL) {
>
> regards,
> Anders Widell
>
> 2013-07-29 13:50, Nagendra Kumar skrev:
>> Hi,
>>      i_su->list_of_susi has list of susi. m_AVD_SU_SI_TRG_DEL calls 
>> avd_susi_delete, which in tern separate susi from i_su->list_of_susi and 
>> deletes it.
>>
>> Code is below:
>>           /* now delete it from the SU list */
>>           if (p_su_si == NULL) {
>>                   susi->su->list_of_susi = susi->su_next;
>>                   susi->su_next = NULL;
>>           } else {
>>                   p_su_si->su_next = susi->su_next;
>>                   susi->su_next = NULL;
>>           }
>>
>>           /* now delete it from the SI list */
>>           if (p_si_su == NULL) {
>>                   susi->si->list_of_sisu = susi->si_next;
>>                   susi->si_next = NULL;
>>           } else {
>>                   p_si_su->si_next = susi->si_next;
>>                   susi->si_next = NULL;
>>           }
>>
>> And then deletes it. This means  one susi is deleted from from 
>> i_su->list_of_susi. When last susi is deleted from i_su->list_of_susi, then 
>> i_su->list_of_susi becomes null and it exists from 'while loop'.
>>
>> So, I see no problem.
>>
>> Let me know if any further clarifications is required.
>>
>> Thanks
>> -Nagu
>>
>> -----Original Message-----
>> From: Anders Widell [mailto:anders.wid...@ericsson.com]
>> Sent: 29 July 2013 16:12
>> To: opensaf-devel@lists.sourceforge.net
>> Cc: Nagendra Kumar; Hans Feldt; praveen malviya; UABHANO
>> Subject: AMF static code analysis regression in 4.2.4 and 4.3.1
>>
>> Hi!
>>
>> I ran some static code analysis on the release candidates for OpenSAF
>> 4.2.4 and 4.3.1. I got a few regressions towards 4.2.3 and 4.3.0, and I need 
>> your help to analyze the following in avd_sgproc.c. The warning says that 
>> i_su->list_of_susi is used after free(). It is freed by 
>> m_AVD_SU_SI_TRG_DEL(), and then dereferenced by avd_compcsi_delete() in the 
>> next iteration.
>>
>> When I look at the code, I don't understand it at all. Does this loop below 
>> terminate? The loop terminates when i_su->list_of_susi is NULL, but it is 
>> not modified within the loop body! If the loop terminates, it must be 
>> because i_su->list_of_susi is somehow modified as a side-effect of calling 
>> avd_compcsi_delete() or m_AVD_SU_SI_TRG_DEL(). This is a very ugly way 
>> coding!!!
>>
>> Line 1398 - 1408 in osaf/services/saf/avsv/avd/avd_sgproc.c on branch 
>> opensaf-4.3.x (tag 4.3.1RC1):
>> -----------------------------------------
>>            /* Free all the SU SI assignments for all the SIs on the
>>             * the SU if there are any.
>>             */
>>
>>            while (i_su->list_of_susi != AVD_SU_SI_REL_NULL) {
>>
>>                /* free all the CSI assignments  */
>>                avd_compcsi_delete(cb, i_su->list_of_susi, false);
>>                /* Unassign the SUSI */
>>                m_AVD_SU_SI_TRG_DEL(cb, i_su->list_of_susi);
>>            }
>> -----------------------------------------
>>
>> regards,
>> Anders Widell
>>
>
> ------------------------------------------------------------------------------
> See everything from the browser to the database with AppDynamics
> Get end-to-end visibility with application monitoring from AppDynamics
> Isolate bottlenecks and diagnose root cause in seconds.
> Start your free trial of AppDynamics Pro today!
> http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
> _______________________________________________
> Opensaf-devel mailing list
> Opensaf-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/opensaf-devel
>
>


------------------------------------------------------------------------------
See everything from the browser to the database with AppDynamics
Get end-to-end visibility with application monitoring from AppDynamics
Isolate bottlenecks and diagnose root cause in seconds.
Start your free trial of AppDynamics Pro today!
http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to