On 10/14/2015 11:41 AM, Mike Blumenkrantz wrote:
> This commit, along with other recent CID-fixing commits, does not make
> sense in the context of the code's functionality, and it only serves to
> make the code less clear. Please try not to add null checks like this to
> resolve a CID report; marking the CID as a false positive is a completely
> valid strategy.
>

In a case like this, e_comp_zone_number_get CAN return NULL (check the 
source):

6. returned_null: e_comp_zone_number_get returns null (checked 11 out of 
13 times). [show details]

so using it (or any function that potentially can return NULL) inline 
(as was done previously) seems (to me) to be more error prone.

I realize what you are saying wrt the context of the code's 
functionality, however the code's functionality (in this case) would 
appear to need improvement ;) as _bgpreview_viewport_update does not 
bother to check if zone is valid before using it.

So what we have (potentially) here is that NULL is getting passed to the 
viewport_update function, which would then crash when trying to 
dereference zone.

Also, I don't see how assigning the return of e_comp_zone_number_get to 
a variable and then using that variable makes the code any less clear. 
Could perhaps elaborate on how this is less clear now ??

dh

> On Wed, Oct 14, 2015 at 11:02 AM Christopher Michael <devilho...@comcast.net>
> wrote:
>
>> devilhorns pushed a commit to branch master.
>>
>>
>> http://git.enlightenment.org/core/enlightenment.git/commit/?id=d74273f7324c2d32e0710e02dd28f17a7396be55
>>
>> commit d74273f7324c2d32e0710e02dd28f17a7396be55
>> Author: Chris Michael <cp.mich...@samsung.com>
>> Date:   Wed Oct 14 10:59:31 2015 -0400
>>
>>      enlightenment: Make sure we have a zone before calling
>>      _bgpreview_viewport_update
>>
>>      @fix CID1324753
>>
>>      Signed-off-by: Chris Michael <cp.mich...@samsung.com>
>> ---
>>   src/bin/e_widget_bgpreview.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/bin/e_widget_bgpreview.c b/src/bin/e_widget_bgpreview.c
>> index 79932b8..c4dbb20 100644
>> --- a/src/bin/e_widget_bgpreview.c
>> +++ b/src/bin/e_widget_bgpreview.c
>> @@ -324,11 +324,13 @@ _e_wid_cb_bg_update(void *data, int type, void
>> *event)
>>          ((ev->desk_x < 0) || (dd->x == ev->desk_x)) &&
>>          ((ev->desk_y < 0) || (dd->y == ev->desk_y)))
>>        {
>> +        E_Zone *zone;
>>           const char *bgfile;
>>
>> +        zone = e_comp_zone_number_get(dd->zone);
>>           bgfile = e_bg_file_get(dd->zone, dd->x, dd->y);
>>           edje_object_file_set(dd->thumb, bgfile, "e/desktop/background");
>> -        _bgpreview_viewport_update(dd->thumb,
>> e_comp_zone_number_get(dd->zone), dd->x, dd->y);
>> +        if (zone) _bgpreview_viewport_update(dd->thumb, zone, dd->x,
>> dd->y);
>>           eina_stringshare_del(bgfile);
>>        }
>>
>>
>> --
>>
>>
>>
> ------------------------------------------------------------------------------
> _______________________________________________
> enlightenment-devel mailing list
> enlightenment-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
>


------------------------------------------------------------------------------
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to