The goal of my program is to read a list of people from the standard input and be able to write and read them onto binary files. Each person in the list is a tPessoa structure containing three string variables. Since, I'm trying to read an unspecified number of people, I'm using "pointer to pointer" to make an array of structs.
When I execute the compiled script with valgrind --leak-check=full --track-origins=yes, I get the following error:
==1211042== Syscall param write(buf) points to uninitialised byte(s)
==1211042== at 0x4986A77: write (write.c:26)
==1211042== by 0x48FCEEC: _IO_file_write@@GLIBC_2.2.5 (fileops.c:1180)
==1211042== by 0x48FE9E0: new_do_write (fileops.c:448)
==1211042== by 0x48FE9E0: _IO_new_do_write (fileops.c:425)
==1211042== by 0x48FE9E0: _IO_do_write@@GLIBC_2.2.5 (fileops.c:422)
==1211042== by 0x48FDFD7: _IO_file_close_it@@GLIBC_2.2.5 (fileops.c:135)
==1211042== by 0x48F0D8E: fclose@@GLIBC_2.2.5 (iofclose.c:53)
==1211042== by 0x1095A7: EscrevePessoasBinario (ex.c:68)
==1211042== by 0x109704: main (ex.c:107)
==1211042== Address 0x4a9e965 is 21 bytes inside a block of size 4,096 alloc'd
==1211042== at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==1211042== by 0x48F0BA3: _IO_file_doallocate (filedoalloc.c:101)
==1211042== by 0x48FFCDF: _IO_doallocbuf (genops.c:347)
==1211042== by 0x48FEF5F: _IO_file_overflow@@GLIBC_2.2.5 (fileops.c:744)
==1211042== by 0x48FD6D4: _IO_new_file_xsputn (fileops.c:1243)
==1211042== by 0x48FD6D4: _IO_file_xsputn@@GLIBC_2.2.5 (fileops.c:1196)
==1211042== by 0x48F1FD6: fwrite (iofwrite.c:39)
==1211042== by 0x109572: EscrevePessoasBinario (ex.c:60)
==1211042== by 0x109704: main (ex.c:107)
==1211042== Uninitialised value was created by a heap allocation
==1211042== at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==1211042== by 0x109384: LePessoasInput (ex.c:28)
==1211042== by 0x1096EF: main (ex.c:106)
==1211042==
==1211042==
==1211042== HEAP SUMMARY:
==1211042== in use at exit: 0 bytes in 0 blocks
==1211042== total heap usage: 12 allocs, 12 frees, 14,666 bytes allocated
==1211042==
==1211042== All heap blocks were freed -- no leaks are possible
==1211042==
==1211042== For lists of detected and suppressed errors, rerun with: -s
==1211042== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
By the way, I'm leaving my entire code at the end of the post so anyone can make their tests and investigations.
Here's what I have found so far.
Valgrind says I'm trying to operate on an uninitialized value, and I believe that means I could be writing garbage to the binary file and that triggers the error. Valgrind says I'm doing that on line 68, but that doesn't seem to be correct. On line 68, fclose(arquivo) is called and the FILE pointer gets freed (nothing unsual here). So I figured the error is elsewhere in that function and, after commenting out some parts, I found the error happens on if ( fwrite(pessoas[pessoa], sizeof(tPessoa), 1, arquivo) != 1). That makes sense because the error seems to be about writing uninitialized values to the binary file.
Valgrind also says the unitialized value is being created on line 28, pessoas[(*nPessoas) - 1] = (tPessoa *) malloc(sizeof(tPessoa));. On line 28, I allocate memory for another tPessoa struct in the pessoas array. However, I can't see how that struct could be uninitialized, since I pass values to the struct's variables right after allocating memory for it. My suspect I'm allocating 100 bytes for the nome (name in Portuguese) variable, but I don't necessarily pass 99 characters plus the '\0' because not all names are that long. So if a person's name is 15 characters long, that means 16 bytes (counting the '\0') leaving another 84 bytes uninitialized.
Could the problem be the nome variable and its unitialized characters? If so, how can I solve this.
If not, I'll leave the entire code below, so anyone can make their on conclusions. Any suggestions are welcome. I'd also like to have a better understanding on what Syscall param write(buf) points to uninitialised byte(s)` really means. Valgrind errors are sort of enigmatic to me.
P.S.: I'm compiling the code with gcc -g.
The code
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
typedef struct ex
{
char nome[100];
char dataNascimento[11];
char cpf[15];
} tPessoa;
void DesalocaPessoas(tPessoa** pessoas, int nPessoas) {
for (int pessoa = 0; pessoa < nPessoas; pessoa++) {
free(pessoas[pessoa]);
}
free(pessoas);
}
tPessoa** LePessoasInput(int* nPessoas) {
tPessoa **pessoas = (tPessoa**) malloc(sizeof(tPessoa *));
//pessoas[0] = (tPessoa*) malloc(sizeof(tPessoa));
char controle = 0;
while (1)
{
(*nPessoas)++;
pessoas = (tPessoa **) realloc(pessoas, sizeof(tPessoa*) * (*nPessoas));
pessoas[(*nPessoas) - 1] = (tPessoa *) malloc(sizeof(tPessoa));
printf("----------- NOVA PESSOA -----------\n");
printf("Nome: ");
scanf("%[^\n]%*c", pessoas[(*nPessoas)-1]->nome);
printf("Data de nascimento: ");
scanf("%[^\n]%*c", pessoas[(*nPessoas)-1]->dataNascimento);
printf("CPF: ");
scanf("%[^\n]%*c", pessoas[(*nPessoas)-1]->cpf);
while(1){
printf("\nDeseja adicionar outra pessoa? [s/n] ");
scanf("%c%*c", &controle);
printf("\n");
if ( controle == 'n' || controle == 'N' ) break;
else if ( controle == 's' || controle == 'S' ) break;
}
if ( controle == 'n' || controle == 'N' ) break;
}
return pessoas;
}
void EscrevePessoasBinario(tPessoa** pessoas, int nPessoas) {
FILE *arquivo;
int pessoasGravadas = 0;
arquivo = fopen("pessoas.bin", "wb");
if (arquivo == NULL) {
return;
}
for ( int pessoa = 0; pessoa < nPessoas; pessoa++ ) {
if ( fwrite(pessoas[pessoa], sizeof(tPessoa), 1, arquivo) != 1 ) {
fclose(arquivo);
return;
}
pessoasGravadas++;
}
printf("Pessoas gravadas: %d\n", pessoasGravadas);
fclose(arquivo);
}
void ImprimePessoasBinario(int nPessoas) {
FILE *arquivo;
tPessoa* pessoaLida = (tPessoa*) malloc(sizeof(tPessoa));
arquivo = fopen("pessoas.bin", "rb");
if (arquivo == NULL) {
free(pessoaLida);
return;
}
int nPessoasLidas = 0;
for (int pessoa = 0; pessoa < nPessoas; pessoa++) {
if (fread(pessoaLida, sizeof(tPessoa), 1, arquivo) != 1) {
break;
}
nPessoasLidas++;
printf("-----------------------------------\n");
printf("%s\n", pessoaLida->nome);
printf("%s\n", pessoaLida->dataNascimento);
printf("%s\n", pessoaLida->cpf);
printf("-----------------------------------\n");
}
printf("Pessoas lidas: %d\n", nPessoasLidas);
free(pessoaLida);
fclose(arquivo);
}
int main() {
int nPessoas = 0;
tPessoa** pessoas = LePessoasInput(&nPessoas);
EscrevePessoasBinario(pessoas, nPessoas);
ImprimePessoasBinario(nPessoas);
DesalocaPessoas(pessoas, nPessoas);
return 0;
}
Your analysis is correct: the structures tPessoa are allocated via malloc() which does not initialize them. The fields are read using scanf() which only sets the destination bytes for the characters read from stdin and the null terminator. This leaves parts of the structures uninitialized, and when fwrite() writes the whole structure to the file, some of the bytes written are still uninitialized, thus have an undeterminate value, which is what Valgrind protests about.
fwrite actually copies the structure contents to the stream buffer. At some point, the stream buffer in turn gets written to the file by internal functions such as _IO_do_write or other obscure library functions, either indirectly by fwrite() or fclose() (fflush or fseek could flush the stream buffer to disk too.) Valgrind does not complain immediately about fwrite copying uninitialized data, but seems to keep track of these bytes and complains when they are finally used or written to disk.
To prevent the Valgrind message, use calloc() to allocate the structures:
pessoas[(*nPessoas) - 1] = (tPessoa *)calloc(sizeof(tPessoa), 1);
Note also that scanf("%[^\n]%*c", pessoas[(*nPessoas)-1]->nome) and the following calls cannot prevent buffer overflows if the input stream contains lines longer than the destination arrays. You should specify the maximum number of characters to store or use a custom function that can perform the appropriate tests.
Here is a modified version:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
typedef struct tPessoa {
char nome[100];
char dataNascimento[11];
char cpf[15];
} tPessoa;
void DesalocaPessoas(tPessoa **pessoas, int nPessoas) {
for (int i = 0; i < nPessoas; i++) {
free(pessoas[i]);
}
free(pessoas);
}
char *ReadString(const char *prompt, char *dest, size_t size) {
size_t i = 0;
int c;
printf("%s: ", prompt);
fflush(stdout);
while ((c = getchar()) != EOF && c != '\n') {
if (i + 1 < size) {
dest[i++] = (char)c;
}
}
if (size) {
dest[i] = '\0';
}
printf("\n");
// return dest unless at end of file
return (c == EOF && i == 0) ? NULL : dest;
}
tPessoa **LePessoasInput(int *nPessoas_ptr) {
tPessoa **pessoas = NULL;
int nPessoas = 0;
for (;;) {
tPessoa pessoa = { 0 };
printf("----------- NOVA PESSOA -----------\n");
if (!ReadString("Nome", pessoa.nome, sizeof pessoa.nome)
|| !ReadString("Data de nascimento", pessoa.dataNascimento, sizeof pessoa.dataNascimento)
|| !ReadString("CPF", pessoa.cpf, sizeof pessoa.cpf)) {
/* input failure */
break;
}
pessoas = (tPessoa **)realloc(pessoas, sizeof(tPessoa *) * (nPessoas + 1));
pessoas[nPessoas] = (tPessoa *)calloc(sizeof(tPessoa), 1);
*(pessoas[nPessoas]) = pessoa;
nPessoas += 1;
char controle = 'n';
for (;;) {
printf("Deseja adicionar outra pessoa? [s/n] ");
scanf(" %c%*c", &controle);
printf("\n");
if (strchr("nNsSyY", controle))
break;
}
if (controle == 'n' || controle == 'N')
break;
}
*nPessoas_ptr = nPessoas;
return pessoas;
}
void EscrevePessoasBinario(tPessoa **pessoas, int nPessoas) {
int pessoasGravadas = 0;
FILE *arquivo = fopen("pessoas.bin", "wb");
if (arquivo == NULL) {
return;
}
for (int i = 0; i < nPessoas; i++) {
if (fwrite(pessoas[i], sizeof(tPessoa), 1, arquivo) != 1) {
fclose(arquivo);
return;
}
pessoasGravadas++;
}
printf("Pessoas gravadas: %d\n", pessoasGravadas);
fclose(arquivo);
}
void ImprimePessoasBinario(int nPessoas) {
tPessoa pessoa = { 0 };
FILE *arquivo = fopen("pessoas.bin", "rb");
if (arquivo == NULL) {
return;
}
int nPessoasLidas = 0;
for (int i = 0; i < nPessoas; i++) {
if (fread(&pessoa, sizeof(tPessoa), 1, arquivo) != 1) {
break;
}
nPessoasLidas++;
printf("-----------------------------------\n");
printf("%s\n", pessoa.nome);
printf("%s\n", pessoa.dataNascimento);
printf("%s\n", pessoa.cpf);
printf("-----------------------------------\n");
}
printf("Pessoas lidas: %d\n", nPessoasLidas);
fclose(arquivo);
}
int main(void) {
int nPessoas = 0;
tPessoa **pessoas = LePessoasInput(&nPessoas);
EscrevePessoasBinario(pessoas, nPessoas);
ImprimePessoasBinario(nPessoas);
DesalocaPessoas(pessoas, nPessoas);
return 0;
}