On Wed, Oct 14, 2015 at 11:57 AM Christopher Michael < [email protected]> wrote:
> 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 > In the original code, there was no null check on the zone, which implies to the casual reader that there is no possibility that the zone can be null. In this case, that is accurate: zone can never be null during normal operation. Now, however, a null check has been added, which will cause a reader to look through the rest of the file to determine how this is possible. > > > On Wed, Oct 14, 2015 at 11:02 AM Christopher Michael < > [email protected]> > > wrote: > > > >> devilhorns pushed a commit to branch master. > >> > >> > >> > http://git.enlightenment.org/core/enlightenment.git/commit/?id=d74273f7324c2d32e0710e02dd28f17a7396be55 > >> > >> commit d74273f7324c2d32e0710e02dd28f17a7396be55 > >> Author: Chris Michael <[email protected]> > >> 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 <[email protected]> > >> --- > >> 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 > > [email protected] > > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > > > > > > ------------------------------------------------------------------------------ > _______________________________________________ > enlightenment-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > ------------------------------------------------------------------------------ _______________________________________________ enlightenment-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
