About Blender bugs

[bearophile]

Notes on bugs found by PVS-Studio in the Blender source code:
http://www.gamasutra.com/blogs/AndreyKarpov/20120423/169021/Analyzing_the_Blender_project_with_PVSStudio.php

> #define  DEFAULT_STREAM  \
>   m[dC] = RAC(ccel,dC); \
>   \
>   if((!nbored & CFBnd)) { \
>   \
>   ....


No one has implemented this yet :-(
http://d.puremagic.com/issues/show_bug.cgi?id=5409

---------------------------

> void tcd_malloc_decode(....) {
>   ...
>   x0 = j == 0 ? tilec->x0 :
>     int_min(x0, (unsigned int) tilec->x0);
>   y0 = j == 0 ? tilec->y0 :
>     int_min(y0, (unsigned int) tilec->x0);
>   x1 = j == 0 ? tilec->x1 :
>     int_max(x1, (unsigned int) tilec->x1);
>   y1 = j == 0 ? tilec->y1 :
>     int_max(y1, (unsigned int) tilec->y1);
>   ...
> }
> 
> 
> 
> This code was most likely created through the Copy-Paste 
> technology
> and the programmer forgot to change the name of one variable 
> during
> editing. This is the correct code:
> 
> y0 = j == 0 ? tilec->y0 :
>   int_min(y0, (unsigned int) tilec->y0);


To avoid such mistakes maybe the IDE has to offer a way to create 
such nearly-repeated lines of code, maybe writing a pattern like 
this:

   $ = j == 0 ? tilec->$ :
     int_min($, (unsigned int) tilec->$);

And then giving a list like [x0, y0, x1, y1] to fill it four 
times.
Dynamic languages are maybe able to do the same.

Or in D with something like:

foreach (s; TypeTuple!("x0", "y0", "x1", "y1")) {
     mixin("  $ = j == 0 ? tilec->$ :
               int_min($, (unsigned int) tilec->$);".replace("$", 
s));

But it's not very readable, and replace() doesn't care if $ is 
present inside strings, where it must not be replaced.

-------------------------------

> #define cpack(x) \
>   glColor3ub( ((x)&0xFF), (((x)>>8)&0xFF), (((x)>>16)&0xFF) )
> static void star_stuff_init_func(void)
> {
>   cpack(-1);
>   glPointSize(1.0);
>   glBegin(GL_POINTS);
> }
> 
> PVS-Studio's warning: V610 Unspecified behavior.
> Check the shift operator '>>. The left operand '(- 1)' is
> negative. bf_editor_space_view3d view3d_draw.c 101
> 
> According to the C++ language standard, right shift of
> a negative value leads to unspecified behavior. In practice
> this method is often used but you shouldn't do that: it
> cannot be guaranteed that the code will always work as
> intended.
> 
> I suggest rewriting this code in the following way:
> 
> cpack(UINT_MAX);


Assigning -1 to an unsigned integer is a garbage way to code, 
expecially in D where uint.max/typeof(T).max are available with 
no need for imports.

-------------------------------

This kind of error is common:

> static bool pi_next_rpcl(opj_pi_iterator_t * pi) {
>   ...
>   if ((res->pw==0)||(res->pw==0)) continue;
>   ...
> }
> 
> PVS-Studio's warning: V501 There are identical
> sub-expressions to the left and to the right of
> the '||' operator: (res->pw == 0) || (res->pw == 0)
> extern_openjpeg pi.c 219


> {
>   ...
>   if ((size_t(lhs0+alignedStart)%sizeof(LhsPacket))==0)
>     for (Index i = alignedStart;i(&lhs0[i]),
>                        ptmp0, pload(&res[i])));
>   else
>     for (Index i = alignedStart;i(&lhs0[i]),
>                        ptmp0, pload(&res[i])));
>   ...
> }
> 
> PVS-Studio's warning: V523 The 'then' statement
> is equivalent to the 'else' statement.

-------------------------------

I have also seen code that is essentially:

size_t x = ...;
if (x < 0) { ... }

It was not a way to disable code. And in D you have /+...+/ and 
version(none){...}.

Bye,
bearophile