Thread: Memory allocation problem, don't know what's wrong

Code:

#include<stdio.h>
#include<stdlib.h>
#include<string.h>
#define NAME_BUFFER_SIZE 80
#define FICHEIRO "registo.bin"

typedef struct {
	char matricula[6];
	char proprietario[NAME_BUFFER_SIZE];
	char apagado;
} veiculo_t;

int main()
{

int opcao = 0, c;
char matricula[10];
FILE *fp;
veiculo_t *memptr = NULL;
veiculo_t registo;

do
{
scanf("%d", &opcao);
getchar();

switch( opcao )
{
case 1:
		fp=fopen(FICHEIRO,"ab+");
		if (fp==NULL) printf("Impossivel aceder ou criar ficheiro");
		else {
			fgets(matricula,sizeof(matricula),stdin);
			memcpy(registo.matricula, matricula, 6);
			fwrite(registo.matricula, sizeof(registo.matricula),1,stdout);
		fgets(registo.proprietario,sizeof(registo.proprietario),stdin);
			registo.apagado='0';
			fwrite(&registo, sizeof(registo),1,fp);
			fclose(fp);
			fflush(fp);
			break;
		}
case 2: 
		fp=fopen(FICHEIRO,"rb");
		if (fp==NULL) printf("Impossivel aceder ou criar ficheiro\n\n");
		else {
			c=0;
			while(fread(&registo,sizeof(registo),1,fp) != 0)
			{
				memptr = realloc(memptr, c*sizeof(registo));
				memcpy(memptr[c].matricula, registo.matricula, 6);
				memcpy(memptr[c].proprietario, registo.proprietario, sizeof(registo.proprietario));
				c++;
			}
			fclose(fp);
			fflush(fp);
		}

		break;
case 3:
		printf("\n\nPrograma vai fechar!\n\n");
		break;
default:
		printf("\nOpcao invalida!!\n\n");
		break;
}
}while (opcao != 3);



return(0);
}

Case 1 saves a license plate (matricula) and a name (proprietario) to a file

Case 2 reads the entire file and puts everything in memory. I'm having a problem here:

Code:

memptr = realloc(memptr, c*sizeof(registo));

As soon as it goes a second time through the cycle I get this error:

Code:

*** glibc detected *** ./ex1: realloc(): invalid next size: 0x0804a170 ***

It has something to do with this:

Code:

memcpy(memptr[c].proprietario, registo.proprietario, sizeof(registo.proprietario));

Because if I remove it I no longer get the error, for some reason why I try to put the name in memory it messes it up. I've already tried using strcpy instead, strlen instead of sizeof, even manually putting the amount of bytes to copy, always the same error.

Anyone have any idea what's wrong and what should I do instead?

The problem is with realloc(), which should always be used with caution as well as care. You have several options in fixing this, but all of which ultimately should be handled with a function that does something like this:

Code:

void *safe_realloc(void *p, size_t currentSize, size_t requestedSize)
{
  void *cpy = realloc(p, requestedSize);
  if(!cpy)
  {
     cpy = malloc(requestedSize);
     if(cpy)
       memcpy(cpy, p, currentSize);
  }

  if(!cpy)
    fprintf(stderr, "Could not reallocate the buffer"); // well... whatever you need it to say anyway
  else
     p = cpy;

  return p;
}

I may miss something but it seems you allocate nothing during the first loop
and thus overwrite memory with memcpy() later:

Code:

			c=0;
			while(fread(&registo,sizeof(registo),1,fp) != 0)
			{
				memptr = realloc(memptr, c*sizeof(registo)); // <-> memptr=realloc(0,0) ???
				...

I forgot to sight the point of the above. realloc() can return NULL. You never make sure it doesn't. Also, it never hurts to make a buffer overly big. Allocating memory is about as slow as things get.

Oh man... How could I have missed the 0*sizeof

God I feel so embarrassed : p

I'm having another problem now, I switched the code that loads to the memory to a separate function, this one:

Code:

void memoria(FILE *fp, veiculo_t registo, veiculo_t *memptr)
{
		fp=fopen(FICHEIRO,"rb");
		if (fp==NULL) printf("Impossivel aceder ou criar ficheiro\n\n");
		else {
			int c=0;
			while(fread(&registo,sizeof(registo),1,fp) != 0)
			{
				memptr = realloc(memptr, (c+1)*sizeof(registo));
				memcpy(memptr[c].matricula, registo.matricula, 6);
				memcpy(memptr[c].proprietario, registo.proprietario, sizeof(registo.proprietario));
				memptr[c].apagado = registo.apagado;
				c++;
			}
			printf("\nFicheiro Carregado para a Memoria\n\n");
			fclose(fp);
			fflush(fp);
		}
}

And in main, I call it as soon as the program opens it:

Code:

int main()
{
int opcao;
FILE *fp;
veiculo_t registo;
veiculo_t *memptr = NULL;

memoria(fp, registo, memptr);

fwrite(memptr[0].matricula, 6,1,stdout);
fwrite(memptr[0].proprietario, strlen(memptr[1].proprietario),1,stdout);

I get a segmentation fault on those fwrites in main(), I know the problem is somewhere in the use of pointers but isn't the memptr supposed to be passed as a pointer to the function?

Originally Posted by vart

I do not see a reason to pass veiculo_t registo as a parameter... It used as if it is local var for this function

Hum, you´re right, and if I'm to return memptr then I don't need veiculo_t *memptr either, Might as well declare it locally.

Thanks for all the help guys.