Opened 11 years ago

Closed 10 years ago

#210 closed defect (fixed)

Build warnings from RTC

Reported by: MatthewWhiting Owned by: MatthewWhiting
Priority: normal Milestone: Release-1.6
Component: Building/Installation Version: 1.5
Severity: normal Keywords:
Cc:

Description

Email from Ben:

Reported on the RTC:

src/ATrous/filter.cc: In member function 'void duchamp::Filter::loadSpline()':
src/ATrous/filter.cc:173:72: warning: iteration 8u invokes undefined behavior [-Waggressive-loop-optimizations]
     for(int i=0;i<12;i++)(*this->sigmaFactors[2])[i] = sigmaFactors3D[i]; 
                                                                        ^
src/ATrous/filter.cc:173:5: note: containing loop
     for(int i=0;i<12;i++)(*this->sigmaFactors[2])[i] = sigmaFactors3D[i]; 
     ^
src/ATrous/filter.cc: In member function 'void duchamp::Filter::loadHaar()':
src/ATrous/filter.cc:238:72: warning: iteration 9u invokes undefined behavior [-Waggressive-loop-optimizations]
     for(int i=0;i<12;i++)(*this->sigmaFactors[2])[i] = sigmaFactors3D[i]; 
                                                                        ^
src/ATrous/filter.cc:238:5: note: containing loop
     for(int i=0;i<12;i++)(*this->sigmaFactors[2])[i] = sigmaFactors3D[i]; 
     ^
src/ATrous/filter.cc:231:72: warning: iteration 7u invokes undefined behavior [-Waggressive-loop-optimizations]
     for(int i=0;i<12;i++)(*this->sigmaFactors[1])[i] = sigmaFactors2D[i]; 
                                                                        ^
src/ATrous/filter.cc:231:5: note: containing loop
     for(int i=0;i<12;i++)(*this->sigmaFactors[1])[i] = sigmaFactors2D[i]; 
     ^
src/ATrous/filter.cc:225:72: warning: iteration 7u invokes undefined behavior [-Waggressive-loop-optimizations]
     for(int i=0;i<19;i++)(*this->sigmaFactors[0])[i] = sigmaFactors1D[i]; 
                                                                        ^
src/ATrous/filter.cc:225:5: note: containing loop
     for(int i=0;i<19;i++)(*this->sigmaFactors[0])[i] = sigmaFactors1D[i]; 
     ^
src/ATrous/filter.cc: In member function 'void duchamp::Filter::loadTriangle()':
src/ATrous/filter.cc:208:72: warning: iteration 9u invokes undefined behavior [-Waggressive-loop-optimizations]
     for(int i=0;i<12;i++)(*this->sigmaFactors[2])[i] = sigmaFactors3D[i]; 
                                                                        ^
src/ATrous/filter.cc:208:5: note: containing loop
     for(int i=0;i<12;i++)(*this->sigmaFactors[2])[i] = sigmaFactors3D[i]; 


As an example:

    double sigmaFactors3D[8] = {1.00000000000,9.56543592e-1,1.20336499e-1,3.49500154e-2,
                1.18164242e-2,4.13233507e-3,1.45703714e-3,5.14791120e-4};
    this->sigmaFactors[2]->resize(8);
    for(int i=0;i<12;i++)(*this->sigmaFactors[2])[i] = sigmaFactors3D[i];


Note that at index location > 7 sigmaFactors3D[i] will be undefined, and possibly even an invalid memory location.

Should really avoid these magic numbers repeated, use a single const or var to avoid duplication.

Change History (3)

comment:1 Changed 11 years ago by MatthewWhiting

Status: newassigned

comment:2 Changed 11 years ago by MatthewWhiting

Better (more O-O) way of doing this would be to have an abstract Filter base class and then derive specific classes for each of the three filters.

This has been added in [1339], along with a FilterFactory? class to do the selection. Need a little bit more thought as to whether the way I've got the filter coefficients referenced is correct, but will probably do for now.

Also, Param::reconFilter is now a pointer, but the interface to it remains via a reference.

comment:3 Changed 10 years ago by MatthewWhiting

Resolution: fixed
Status: assignedclosed

Note that we have changed the way the reconFilter is instantiated in Param. Rather than doing

this->reconFilter.define(this->filterCode);

we now do

FilterFactory filtFac;
this->reconFilter = filtFac.getFilter(this->filterCode);

Closing ticket, as things are building OK now.

Note: See TracTickets for help on using tickets.