-------- Original-Nachricht -------- > Datum: Sat, 12 Feb 2011 02:37:48 +0000 > Von: Campbell Barton <[email protected]> > An: bf-blender developers <[email protected]> > Betreff: Re: [Bf-committers] bugs?
> You suggest we should eventually make blender warning free, I'd like > this too but at the moment there are a enough false positives mixed in > with Clang's warnings. > Or, at least warnings which will never happen unless blender runs into > memory corruptions/un-thread-safe code. > In practice this isn't so simple. Completely agree here. Working code is more important than nice looking code. > Examples: > Clang's static checker considers that checks on a value which is a > pointer passed to the argument may not be the same twice: Example, see > checks on (mface[i].flag & ME_SMOOTH) > http://www.graphicall.org/ftp/ideasman42/2011-01-07-2/report-dOTWay.html#EndPath > > In this case it can be silenced by assigning a const and checking that > instead: > const int smoothnormal = (mface[i].flag & ME_SMOOTH); > if(smoothnormal)... > ... > if(smoothnormal)... I think Clang also has a point here. If you are really sure that no other threads can change mface[i], because you know there is a lock a few stack frames above, then this really is a false-positive warning. I think your suggestion also results in code that looks nicer because you have a talking name for the condition. > ... you can see its unreasonable to assign const's everywhere just > because a variable could theoretically be changed by another thread > and cause a check to be true once and false the next. That also depends on how much more you know than Clang. I'll give you an example why I think consts are a good idea. This is a reocurring error I frequently make at work. I have to work a lot with pre-allocated arrays in real-time processing applications. int n = something.nThings; for( int i=0; i<n; ++n) do_it( something.things[i]); Code compiles without a single warning. See the problem? The loop increments n instead of i and happily runs 2^32 iterations on i=0. const int n = something.nThings; for( int i=0; i<n; ++n) do_it( something.things[i]); Compile it and you get an error for incrementing n. Same goes for = instead of ==. > I didn't mean to bring this up since blender's code has lots of > potential buffer overruns, every so often I try to reduce these by > using BLI_snprintf and similar but we still have enough unchecked uses > of sprintf, strcpy(). > Of course these _should_ be changed, on the other hand there are lots > of bugs in the tracker and hardly any are to do with buffer overruns > because over time we fixed reported cases. Agree here. Don't change it unless you know its broken. > Since you point this out, I changes the function so the values are > printed directly then the stdout flushed. r34785. Saw the final printf after I send my reply. > This is easier then imposing string lengths. In this case. > Using (void)value; isn't that strange, Nokia's QT-Toolkit uses a macro > for this to silence unused argument warnings. > #define Q_UNUSED(x) (void)x; > > We could #ifdef a macro like this for MSVC is needed. > Not suggesting we add this but I'd prefer this to removing assignments > potentially adding confusion later. For a lot of copy-and-paste code in the UI, this is the more practical approach, I agree. As closing words, I would like to add a big "Thank you" for doing this task of cleaning up these warnings. -- Dr. Lars Krueger Empfehlen Sie GMX DSL Ihren Freunden und Bekannten und wir belohnen Sie mit bis zu 50,- Euro! https://freundschaftswerbung.gmx.de _______________________________________________ Bf-committers mailing list [email protected] http://lists.blender.org/mailman/listinfo/bf-committers
