useless verifications in r35_pack
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
Cuneiform for Linux |
New
|
Undecided
|
Unassigned |
Bug Description
This is about a cycle in the function r35_pack in the file r35.c. Of course, the same applies to the function r35_pack_gray.
for(t=kx=
430 {
431 for(num=
432 { /* first lines in strip */
433 compress_line( &rast[t],dx, buf_comp,
434 addcomp(
435 }
436 if( num!=dy )
437 { /* last line in strip */
438 compress_line( &rast[t],dx, buf_comp,
439 if( Yval[k+1] )
440 { /* intrsected line */
441 ALL_addcomp(
442 ALL_addcomp(
443 }
444 else /* alone line */
445 addcomp(
446 }
447 }
Look at the very first if. Clearly, the value of num here is Ycut[k+1]. According to the definition of the array Ycut (which is given by the function Makescale), Ycut[k+1]!=dy if and only if k+1<TO_Y. Hence, we can mostly avoid this verification inside the cycle. This also allows us to get rid of some variables. I suggest to rewrite the cycle as the following:
for(k=0;
{
for(num=
{ /* first lines in strip */
compress_line( &rast[num*d_x],dx, buf_comp,
addcomp(
}
/* last line in strip */
compress_line( &rast[num*d_x],dx, buf_comp,
if( Yval[k+1] )
{ /* intrsected line */
ALL_
ALL_
}
else /* alone line */
addcomp(
}
for(num=
{ /* first lines in strip */
compress_line( &rast[num*d_x],dx, buf_comp,
addcomp(
}
Having additional sanity checks is not a bad thing unless they complicate the code needlessly.
Rather than getting rid of this completely, maybe we should convert it to an assert? This would document the invariant expected by the function.