0

I'm quite new to c++ and I don't manage to make this works. Sorry but i have always worked with languages that didn't helped me to think in terms of memory pointers ans so maybe this is a fooly question.

I want to pass a float array as a default parameter. Like this:

void getHistogram(const Mat& src, MatND& hist, float range[]=NULL) {

    if(range==NULL) {
        double maxPixel=0;
        minMaxLoc(src, 0, &maxPixel, 0, 0);
        range = { 0, maxPixel +1 };
    }

    // now calculate histogram with the right range
    // something something
}

I have tried with some different syntax but i'm always in front on some errors like

histogram.cpp:21: warning: extended initializer lists only available with -std=c++0x or -std=gnu++0x
histogram.cpp:21: error: cannot convert ‘<brace-enclosed initializer list>’ to ‘float*’ in assignment

EDIT (but with memory leak):

Ok, thatnks to this answer i have resolved in this way:

void imHist(const Mat& src, MatND& hist, float range[]=NULL) {

    if(range==NULL) {
        double maxPixel=0;
        minMaxLoc(src, 0, &maxPixel, 0, 0);

        range = new float[2];
        range[0] = 0;
        range[1] = maxPixel +1;
    }

}

some pros or cons?

EDIT 2

see the accepted answer

Community
  • 1
  • 1
nkint
  • 11,513
  • 31
  • 103
  • 174

2 Answers2

5

Replace your current code …

void getHistogram(const Mat& src, MatND& hist, float range[]=NULL) {

    if(range==NULL) {
        double maxPixel=0;
        minMaxLoc(src, 0, &maxPixel, 0, 0);
        range = { 0, maxPixel +1 };
    }

    // now calculate histogram with the right range
    // something something
}

with this:

void getHistogram(const Mat& src, MatND& hist, float range[] ) {
    assert( range != 0 );
    // now calculate histogram with the right range
    // something something
}

void getHistogram(const Mat& src, MatND& hist ) {
    double maxPixel=0;
    minMaxLoc(src, 0, &maxPixel, 0, 0);
    float range[] = { 0, maxPixel +1 };
    getHistogram( src, hist, range );
}

That said, why are you using float instead of double?


EDIT: the OP explains that the array of float is required by OpenCV.

He further explains in his answer that he's resolved the problem as follows:

void imHist(const Mat& src, MatND& hist, float range[]=NULL) {

    if(range==NULL) {
        double maxPixel=0;
        minMaxLoc(src, 0, &maxPixel, 0, 0);

        range = new float[2];
        range[0] = 0;
        range[1] = maxPixel +1;
    }
}

This leaks memory, and is needlessly inefficient.

EDIT 2: the reason that the above code leaks memory is because there is a new (which allocates memory), but no corresponding delete expression (which frees the memory), and there is no indication of whether the memory was allocated by new or provided by the caller.

The reason that it's inefficient is that a dynamic memory allocation typically is orders of magnitude slower than e.g. a basic assignment or stack allocation, because it has to do a search for a suitable small free chunk of memory.

The stack allocation (C++ “automatic memory”) avoids that inefficiency by always deallocating in reverse order of allocation, so that it can always use the start of the free stack memory area for the next allocation (modulo direction: in practice the stack grows downward in memory on all machines I have known about).

Cheers and hth. - Alf
  • 142,714
  • 15
  • 209
  • 331
  • i am using float because the function calHist in OpenCV 2.4 accept float value as range – nkint Jan 09 '13 at 11:07
  • ok i haven't seen the answer edits, sorry. Can you explain me why there is a memory leak? – nkint Jan 09 '13 at 11:43
1

When you pass an array, you pass a pointer to the first element, not all the array. For give you a responce, could you edit your post and add the code when you call the function?

Vargan
  • 1,277
  • 1
  • 11
  • 35
  • Ok, so now it's working? By the way, you need at any cost an array of float, or you can choose everything you want for your implementation? – Vargan Jan 09 '13 at 11:15
  • 1
    I suggest to use the solution of Cheers and hth. - Alf, and divide the cases (Null or not), that solution avoid memory leak and is clear to read. – Vargan Jan 09 '13 at 11:31
  • You need to be careful when you use the keyword "new". For every new there must exists a delete. Of course you can use your implementation, but remember to make your destructor as well. – Vargan Jan 09 '13 at 11:52