Valgrind problem and dynamic allocation in C++

Question

I have a program a little bigger than that, I selected the part that I can not solve, are errors that seem unfounded, I’ve really tried everything. I imagine it’s a structural problem.

I will post the code I selected, it is not the whole program but loads my problem

 #include <iostream>
 #include <fstream>
 #include <cstring>
 #include <cctype>
 #include <cstdio>

 using namespace std;

 struct Agregado
 {
    char *chave;
    char *media;
    double n;
  };

  int estrutura_e_ordena(ifstream &fin, int registros, int posicaoChave, int 
  posicaoMedia)
  {
    string linha;
    Agregado *reg = new Agregado[registros]();
    int indice = 0;
    int j;
    int cont = 0;
    for (indice = 0; fin.peek() != EOF; indice++)
    {
    for (j = 0; j < registros && fin.peek() != EOF; j++)
    {

        getline(fin, linha);
        int t = strlen(linha.c_str()) + 1;
        char *c = new char[t];
        strcpy(c, linha.c_str());

        reg[j].media = new char[t];
        reg[j].chave = new char[t];

        reg[j].chave = strtok(c, ", ");


        for(int i=1; i<=posicaoChave; i++){
            reg[j].chave = strtok(NULL, ", ");
        }    


        reg[j].media = strtok(c, ", ");
        for(int i=1; i<=posicaoMedia; i++){
            reg[j].media = strtok(NULL, ", ");
        }

        delete[] c;
    }
    for (int i = 0; i < j; i++)
    {

        delete[] reg[i].media;
        delete[] reg[i].chave;
    }
    j = 0;
}

fin.close();
delete[] reg;
return indice;
}

int main(int argc, char **argv)
{

char *nome = new char[strlen(argv[1]) + 1];
strcpy(nome, argv[1]);

int registros = atoi(argv[2]);
char *chave = new char[strlen(argv[3]) + 1];
strcpy(chave, argv[3]);

char *media = new char[strlen(argv[4]) + 1];
strcpy(media, argv[4]);

ifstream fin(nome);

int posicaoChave = 0, posicaoMedia = 1;

estrutura_e_ordena(fin, registros, posicaoChave, posicaoMedia);
delete[] nome;
delete[] chave;
delete[] media;
fin.close();
return 0;
}

Valgrind points out the following errors`

==1275== Invalid free() / delete / delete[] / realloc()
==1275==    at 0x483758B: operator delete[](void*) (vg_replace_malloc.c:641)
==1275==    by 0x1095AD: estrutura_e_ordena(std::basic_ifstream<char, 
std::char_traits<char> >&, int, int, int) (saco.cpp:55)
==1275==    by 0x109755: main (saco.cpp:82)
==1275==  Address 0x4d44150 is 0 bytes inside a block of size 6 free'd
==1275==    at 0x483758B: operator delete[](void*) (vg_replace_malloc.c:641)
==1275==    by 0x1094F5: estrutura_e_ordena(std::basic_ifstream<char, 
std::char_traits<char> >&, int, int, int) (saco.cpp:49)
==1275==    by 0x109755: main (saco.cpp:82)
==1275==  Block was alloc'd at
==1275==    at 0x483650F: operator new[](unsigned long) 
(vg_replace_malloc.c:423)
==1275==    by 0x109369: estrutura_e_ordena(std::basic_ifstream<char, 
std::char_traits<char> >&, int, int, int) (saco.cpp:30)
==1275==    by 0x109755: main (saco.cpp:82)`

I don’t have much experience with Valgrind, but it points out errors like "Invalid free() / delete / delete[] / realloc()" that don’t make any sense, I just allocate, I operate the vector and then desaloco. Note: I’m using exactly what I can use, I can’t use string except to read the line and copy to the char array, and I can’t use object-oriented functions and libraries.

In context, this project is an external ordering job, and if you want to test, the input I’m using is

A, N1
a, 0.501731038000827
b, 0.509714268041373
c, 0.501163287116723
d, 0.503026776283879
e, 0.500950413375161
f, 0.502142644681647
g, 0.501214116159229
h, 0.501854463314318
i, 0.496600248440377
j, 0.493076243330675
k, 0.504138789431106
l, 0.498207171945015
m, 0.496285022266796
n, 0.501737614492674
o, 0.500618404020615
p, 0.498576392529170
q, 0.499085967949642
r, 0.509327587184305
s, 0.500491417712932

The arguments in the command line are, 1 = input name, 2 = number of lines I can read at a time, recommend 6. 3 = aggregation key, in case A. 4 = average column (N1), numbers that, in another part of the project, will serve to take an average according to the selected aggregation key.

Answer 1

Let me start by saying that you’ve made a lot of trouble using char * everywhere when std::string was much simpler, avoiding having to manage the memory of each string individually, as well as its terminators.

The problem is actually due to strtok, who does something different than you imagined.

Now watch out for the documentation:

... Return Value: If a token is found, a Pointer to the Beginning of the token.

Namely the strtok returns a pointer to the beginning of the found word(token). So let’s start with memory allocation:

reg[j].chave = new char[t];

In this first statement, allocate an array of chars with size t and saves the pointer for that array in reg[j].chave.

But right away you do this:

reg[j].chave = strtok(c, ", ");

Here it guards over the original pointer which is valid to do delete, a pointer to a character in the middle of the string.

Just below when it does:

delete[] reg[i].chave;

It already becomes a delete invalid because it no longer points to the beginning of the string, but to the middle/end of the string. The same is true for the reg[i].media.

My proposal for a solution:

Of course you can always save the original pointer to another variable and do delete in this, but what I really propose to you as a solution (which I will not demonstrate here in this answer due to the extension) is to rewrite everything but now using the appropriate of c++.

This means changing the char* for std::string, and make a split c++ version using for example find and replace. Have an example of split thus in Soen. There are also other alternatives such as using the 3rd parameter of the getline starting from a stream, or use a stringstream if to be separated by spaces.

With this change all the code you have here in the question reduces to less than half, and is much simpler to notice and maintain, as well as avoid memory management failures since the string already manages the memory of your characters internally.