www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - About Blender bugs

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
Apr 23 2012