memory leak strtok & atof

Question

int fromVectoEigen(vector<string> & source, double ** & array, int & n)
{  
    cout<<"n:"<<n<<'\n';
    int counter = 0;

    for (int j = n-1 ; j >= 0 ; j--) // iterate through each line
    {
        string mystring = source.back(); // take last line

        counter++;
        char * str = new char [mystring.length()+1];
        std::strcpy (str, mystring.c_str()); // copy string to char array
        char * pch; // pointer to tokens
        pch = strtok (str," ,-");
        for( int i = 0 ; i<3 ; i++) // dismiss first 3 columns
        {

            pch = strtok (NULL, " ,-");
        }

        for (int i= 0 ; i<n ; i++)
        {
            double val = atof(pch); // cast char to double

            //array[j][i]= val;
            //cout<<array[j][i]<<'\t';
            //pch = strtok (NULL, " ,-");
        }
        //
        //source.pop_back();

        }

    return 0;
}

Hey!

With this function I want to read in a matrix from file to an twodimensional array The matrix has as many lines as columns. And i want to split the lines by space with strtok. The lines were already read into a vector in another function.(which worked - itested it). So I dont know the Problem because I tried to run this with a small matrix 4 columns 4 lines, which worked perfectly! Now I wanted to try it with a big matrix which has over 1000 lines and as many columns. and I get this error message from valgrind:

Invalid read of size 1 ==26501== at 0x58A87AB: __strtod_l_internal (strtod_l.c:538) ==26501== by 0x4015BB: fromVectoEigen(std::vector >&, double**&, int&) (topo.cpp:70) ==26501== by 0x40362B: main (main.cpp:36) ==26501== Address 0x0 is not stack'd, malloc'd or (recently) free'd

I tried the comment/uncomment method and everything works fine up to where i use atof....which I dont understand cause it worked fine with the small matrix

The values in the big matrix look like this: 0.11991517 where as in the small test matrix I only had values like 0 or 0.1

I hope I explained enough...pls ask for more detail if needed.

Answer 1

You allocate str, but you never free it ... and with the code you have, you never can. Thus, every time you call this function, you will leak memory.

You should either add a delete [] str; when you are done with it, or don't use dynamic memory at all and stick with std::string.

Additionally, you can avoid the whole conversion issue and just use std::istream::operator>> (e.g. using std::istringstream) to parse your input.

Something like this (untested) should work better for you:

struct reader : std::ctype<char>
{
    reader() : std::ctype<char>(get_table()) {}

    static std::ctype_base::mask const* get_table()
    {
        static std::vector<std::ctype_base::mask> rc(table_size, std::ctype_base::mask());
        rc['\n'] = std::ctype_base::space;
        rc[','] = std::ctype_base::space;
        rc['-'] = std::ctype_base::space;
        rc[' '] = std::ctype_base::space;
        rc['\t'] = std::ctype_base::space;
        return &rc[0];
    }
};

int fromVectorEigen(const std::vector<std::string>& source, std::vector<std::vector<double>>& destination)
{
    reader rdr;
    std::for_each(source.rbegin(), source.rend(), [&](const std::string& s)
    {
        std::istringstream iss(s);
        iss.imbue(std::locale(), &rdr);
        std::string ignored_columns;
        double value;
        iss >> ignored_columns >> ignored_columns >> ignored_columns;
        std::vector<double> values;
        while (iss >> value)
        {
            values.push_back(value);
        }
        destination.push_front(values);
    });
    return 0;
}

Answer 2

If you want to stay with this current design of not using streams, but eliminate the explicit allocation of memory, you could use a std::vector:

  #include <vector>
  //...
  std::vector<char> str(mystring.begin(), mystring,end());
  str.push_back(0);
  char * pch; // pointer to tokens
  pch = strtok (&str[0]," ,-");