On 06/01/14 14:41, Tom Hacohen wrote: > On 06/01/14 14:19, Daniel Juyung Seo wrote: >> On Mon, Jan 6, 2014 at 6:21 PM, David Seikel <onef...@gmail.com> wrote: >> >>> On Mon, 6 Jan 2014 09:31:59 +0100 Stefan Schmidt >>> <ste...@datenfreihafen.org> wrote: >>> >>>> clang scan-build: >>>> o EFL scan-build reports 484 (483) issues. >>>> >>> https://build.enlightenment.org/job/nightly_efl_clang_x86_64/lastSuccessfu >>>> lBuild/artifact/scan-build/build/ >>>> o Elementary scan-build reports 121 (119) >>> >>> Elementary reports 121 of what? Hard to tell if this is good or bad. >>> I would guess it's "issues" like the EFL version just above it. Should >>> state that. >>> >>> >> It looks like there are many false alarms but can't mark it as false alarm >> like coverity. >> For example, in internal function, we used ELM_WIDGET_DATA_GET(obj, sd) and >> clang reported "dereference of null pointer" on sd->resize_obj. >> But sd should not be null inside internal functions and checking null will >> hide the root problem. >> Is there a way to fix the clang build or we should just follow the reports? >> >> Anyhow, I found some useful reports from clang. >> Thanks clang! >> > > We shouldn't silence clang. I just checked the code, and clang is right. > The reason why clang sees a problem is because this check is wrong and > should not be done, and the macro should be fixed. > > I assume the author was trying to verify that the functions are not > being called on objects of different classes. This check is stupid > because it could never happen. Those are implementations of functions > for a specific class. The only way those functions will ever be called, > is if that function was called for an object of the correct class. > This check is valid in some cases (when calling on subojects), and if > used in the legacy API (just the isa, no need for the sd get there), it > might help spot issues. However even when used with the legacy API it's > not that helpful, because most issues will be caught anyway by trying to > call the wrong function for a class (eo_do will catch that). > > The right course of action is to identify the cases in which it's used > appropriately (on subojects), and keep them, and only them. I'll take a > look now, and will get it in (I think it's OK to put in in the > stabilization period, as it's wrong and should be fixed, but it's not > critical. I guess it's up for debate).
Done and pushed. I guess clang might still complain as it'd think that function might return null (no idea if it's that sophisticated), but the problem I found there is fixed. Also, I checked, and there's already API_ENTRY for checking what that macro was supposedly checking, and I checked other widgets, and there things seem to be done correctly (check on legacy api entry, but not internally). -- Tom. ------------------------------------------------------------------------------ Rapidly troubleshoot problems before they affect your business. Most IT organizations don't have a clear picture of how application performance affects their revenue. With AppDynamics, you get 100% visibility into your Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro! http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
