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