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