Problem with valgrind and dynamic allocation in C++
I have a program somewhat larger than this, I have selected the part that I can not solve, are errors that seem unfounded, I have really tried everything. I imagine it's a structural problem.
I will post the code I selected, it is not the entire 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 to 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 me to errors like " Invalid free () / delete / delete [] / realloc ()" which makes no sense at all, I just allocate, operate the vector and then deallocate. Note: I'm using exactly what I can use, I can't use string except to read the line and copy to the Char vector, and I can't use object-oriented functions and libraries.
Contextualizing, this project is an external sort job, and in case 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 on the command line are, 1 = input name, 2 = number of lines I can read at a time, I 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.
1 answers
I'll start by saying that you've complicated it quite a bit, 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, which does something different than you imagined.
Now look at the documentation :
... Return Value: If a token is found, a pointer to the beginning of the token.
I.e. strtok returns you a pointer to the start of the word(token) found.
So let's start with memory allocation:
reg[j].chave = new char[t];
In this first statement, allocates an array of chars with size t and saves the pointer to that array at reg[j].chave.
But then it does this:
reg[j].chave = strtok(c, ", ");
Here saves above the original pointer, which is valid to make delete, a pointer to a character in the middle of the string.
Logo further down when it does:
delete[] reg[i].chave;
Already becomes an invalid delete because it no longer points to the beginning of the string, but to the middle/end of it. The same goes for reg[i].media.
My solution proposal:
Of course you can always save the original pointer in another variable and do delete in that one, but what I really propose as a solution (which I will not demonstrate here in this answer due to the extension) is to rewrite everything but now using the suitable for c++.
This means exchanging the char* for std::string, and making a split C++ version using for example find and substr. has an example of split this way in Soen. There are also other alternatives such as using the 3rd parameter of getline starting from a stream, or using a stringstream if it is to separate by spaces.
With this exchange all the code that has here in the question reduces to less than half, and it is much simpler to realize and maintain, as well as Avoid memory management failures since the string already manages the memory of its characters internally.