• bearophile (34/108) Apr 23 2012 No one has implemented this yet :-(

"bearophile" <bearophileHUGS lycos.com> writes:

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