Heap Data Messed Up

Question

I have luckily found a lot of useful answers here by reading others' questions but this time I am completely helpless, so I have to raise a question myself:

I try to create a programme that applies a convolution to a data series. For the convolution kernels (=arrays of certain numbers) that have different lengths are needed.

I implement this by using a float** and inserting values at the twice dereferenced variables. The number of arrays is fixed, the length of each array is not, so the "sub-arrays" are allocated by using new—in function CreateKernels after if.

This function then returns that float** together with another pointer bundled as a structure to main.

Now here comes the problem: I've looked at the dereferenced values of my kernel pointers with debugging watches. Everything worked fine and all numbers are as intended after CreateKernels returns (i.e. looked at the memory from the main scope). But after the next command in main my numbers are totally screwed. If I try to use the data in subsequent code I get a segfault.

So what my current reasoning is: As I use new to create the variables they are supposed to be in the heap and should stay there until I free[] the variable—at any rate they should not be confined to the scope of CreateKernels. The assignment of the pointers to the kernel structure and returning it may be strange to some of you but it works. So what actually messes up my data is the next command after CreatKernels. Initialising an int instead of creating a fstream does not mess up my numbers. But why?

Is this something my OS memory management gets wrong? Or is it a stupid programming error? I am running Ubuntu 12.04-64bit and used both Code::Blocks and g++ for compilation (all default settings) and both executables give me a segfault.

I would really appreciate any hint or experience with this issue!

This is the relevant code:

#include <string>
#include <stdlib.h>
#include <iostream>
#include <fstream>
#include <iomanip>
#define HW width/2
#define width1 4       // kernel width-1 (without peak)

using namespace std;

struct kernel_S
{
    const float ** ppkernel;
    const float * pnorm;
};

void BaseKernel(const int & width, float * base) // function that fills up an 1d-array of floats at the location of base
{
    for(int i=0; i<=HW-1; i++)      // fill left half as triangle
    {
        base[i] = i+1;
     }
    base[HW] = HW+1;          // add peak value
    for(int i=HW+1; i<=width; i++)   // fill right half as decreasing triangle
    {
        base[i] = width-i+1;
    }
}

kernel_S CreateKernels(const int &width) // function that creates an array of arrays (of variable length)
{
float base_kernel[width+1];    // create a full width kernel as basis
BaseKernel(width, base_kernel);

float * kernel[width+1];       // array of pointers, at each destination of a pointer a kernels is stored
float norm[width+1];           // norm of kernel

for(int j=0; j<=width; j++)    // fill up those individual kernels
{
    norm[j] = 0;
    if(j<=HW)                   // left side up to peak
    {
        kernel[j] = new float[HW+j+1]; // allocate mem to a new array to store a sub-kernel in
        for(int i=0; i<=HW+j; i++)
        {
            *(kernel[j]+i) = base_kernel[HW-j+i];  //use values from base kernel
            norm[j] += base_kernel[HW-j+i];        // update norm
        }
    }
    else if(j>=HW+1)
    {
        kernel[j] = new float[HW+width-j+2];
        for(int i=0; i<=HW+width-j; i++)
        {
            *(kernel[j]+i) = base_kernel[i];
            norm[j] += base_kernel[i]; // update norm
        }
    }
}

kernel_S result;                // create the kernel structure to be returned
result.ppkernel = (const float **) kernel;  // set the address in the structure to the address of the generated arrays
result.pnorm    = (const float *) norm; // do the same for the norm
return result;
}

int main()
{
    kernel_S kernels = CreateKernels(width1);            // Create struct of pointers to kernel data
    ifstream name_list(FILEPATH"name_list.txt", ios::in);// THIS MESSES UP THE KERNEL DATA

    // some code that would like to use kernels

    return 0;
}

Answer 1

Please see

How do I use arrays in C++?

You return pointers to local stack data (kernel and norm). You should allocate dynamically:

float ** kernel = new float*[width+1];       // array of pointers, at each destination of a pointer a kernels is stored
float *norm = new float[width+1];           // norm of kernel

Remember to delete with delete[].

However, I suggest using std::vector or std::array instead

#include <string>
#include <stdlib.h>
#include <iostream>
#include <fstream>
#include <iomanip>
#include <vector>
#define HW width/2
#define width1 4       // kernel width-1 (without peak)

using namespace std;

typedef std::vector<float> floatV;
typedef std::vector<floatV> floatVV;

struct kernel_S
{
    floatVV kernel;
    floatV  norm;
};

floatV BaseKernel(int width)       // function that fills up an 1d-array of floats at the location of base
{
    floatV base;
    base.resize(width + 1);
    for(int i=0; i<=HW-1; i++)     // fill left half as triangle
    {
        base[i] = i+1;
    }
    base[HW] = HW+1;               // add peak value
    for(int i=HW+1; i<=width; i++) // fill right half as decreasing triangle
    {
        base[i] = width-i+1;
    }
    return base;
}

kernel_S CreateKernels(const int &width)                   // function that creates an array of arrays (of variable length)
{
    const floatV base_kernel = BaseKernel(width);          // create a full width kernel as basis

    kernel_S result;                                       // create the kernel structure to be returned
    result.kernel.resize(base_kernel.size());
    result.norm.resize(base_kernel.size());

    for(int j=0; j<=width; j++)                            // fill up those individual kernels
    {
        result.norm[j] = 0;
        if(j<=HW)                                          // left side up to peak
        {
            result.kernel[j].resize(HW+j+1);               // allocate mem to a new array to store a sub-kernel in
            for(int i=0; i<=HW+j; i++)
            {
                result.kernel[j][i] = base_kernel[HW-j+i]; // use values from base kernel
                result.norm[j] += base_kernel[HW-j+i];     // update norm
            }
        }
        else if(j>=HW+1)
        {
            result.kernel[j].resize(HW+width-j+2);
            for(int i=0; i<=HW+width-j; i++)
            {
                result.kernel[j][i] = base_kernel[i];
                result.norm[j] += base_kernel[i];          // update norm
            }
        }
    }
    return result;
}

int main()
{
    kernel_S kernels = CreateKernels(width1);     // Create struct of pointers to kernel data
    ifstream name_list("name_list.txt", ios::in);

    // some code that would like to use kernels

    return 0;
}

Note If you want kernel and norm to be const inside the result struct, just make the whole struct const:

    const kernel_S kernels = CreateKernels(width1);