Ask Your Question
2

is this code in modules/flann/src/miniflann.cpp a bug?

asked 2017-09-01 16:13:05 -0600

Using an experimental new static analysis technique we (GrammaTech) detected what may be a bug in createIndicesDists, miniflann.cpp:

static void createIndicesDists(OutputArray _indices, OutputArray _dists, 
                                       Mat& indices, Mat& dists, int rows, 
                                       int minCols, int maxCols, int dtype) 
        { 
            if( _indices.needed() ) 
            { 
                indices = _indices.getMat(); 
                if( !indices.isContinuous() || indices.type() != CV_32S || 
                    indices.rows != rows || indices.cols < minCols || indices.cols > maxCols ) 
                { 
                    if( !indices.isContinuous() ) 
                       _indices.release(); 
                    _indices.create( rows, minCols, CV_32S ); 
                    indices = _indices.getMat(); 
                } 
            } 
            else 
                indices.create( rows, minCols, CV_32S ); 

            if( _dists.needed() ) 
            { 
                dists = _dists.getMat(); 
                if( !dists.isContinuous() || dists.type() != dtype || 
                   dists.rows != rows || dists.cols < minCols || dists.cols > maxCols ) 
                { 
                    if( !indices.isContinuous() ) // HERE!  should this be 'dists'?
                        _dists.release(); 
                    _dists.create( rows, minCols, dtype ); 
                    dists = _dists.getMat(); 
                } 
            } 
            else 
                dists.create( rows, minCols, dtype ); 
        }

The block of code operating on _dists/dists appears to be a copy of the above block operating on _indices/indices, with all of the occurrences of _indices/indices changed to _dists/dists except the one noted in the "HERE!" comment. This inconsistency couid be intentional, but it is highly suspicious. Can anyone "in the know" confirm or deny that this is a bug?

edit retag flag offensive close merge delete

Comments

github link of the mentioned code it seems a bug. let us ask developers cc @mshabunin

sturkmen gravatar imagesturkmen ( 2017-09-01 16:29:29 -0600 )edit

Yes, looks like a bug. Please create an issue in the tracker or submit a pull request with the fix.

mshabunin gravatar imagemshabunin ( 2017-09-02 01:31:08 -0600 )edit

Do I need to create another separate account for your bug tracking system, or does the one used for this site cover both? It is surprising that I don't find a link to your bug tracking site on this page...

MysteriousBugOracle gravatar imageMysteriousBugOracle ( 2017-09-03 13:50:39 -0600 )edit
sturkmen gravatar imagesturkmen ( 2017-09-03 14:21:26 -0600 )edit

1 answer

Sort by ยป oldest newest most voted
2

answered 2017-09-03 17:29:38 -0600

This could have been avoided by a little factoring, as follows:

// there might be a better name for this...
static void createIndicesOrDists(OutputArray _array, Mat& mat,
                                 int rows,  int minCols, int maxCols, int dtype) 
{ 
    if( _array.needed() ) 
    { 
        mat= _array.getMat(); 
        if( !mat.isContinuous() || mat.type() != CV_32S || 
             mat.rows != rows || mat.cols < minCols || mat.cols > maxCols ) 
        { 
            if( !mat.isContinuous() ) 
               _array.release(); 
            _array.create( rows, minCols, CV_32S ); 
            mat= _array.getMat(); 
        } 
    } 
    else 
        mat.create( rows, minCols, CV_32S );
}

static void createIndicesDists(OutputArray _indices, OutputArray _dists, 
                               Mat& indices, Mat& dists, int rows, 
                               int minCols, int maxCols, int dtype) 
 { 
       createIndicesOrDists(_indices, indices, rows, minCols, maxCols, dtype);
       createIndicesOrDists(_dists, dists, rows, minCols, maxCols, dtype);
 }
edit flag offensive delete link more

Question Tools

1 follower

Stats

Asked: 2017-09-01 16:13:05 -0600

Seen: 309 times

Last updated: Sep 03 '17