Error when freeing memory (allocated with realloc)

This is a discussion on Error when freeing memory (allocated with realloc) within the C Programming forums, part of the General Programming Boards category; Hi everybody, I am trying to make an adress book where I can store a persons name and his phone ...

  1. #1
    Registered User
    Join Date
    Jan 2008
    Posts
    7

    Error when freeing memory (allocated with realloc)

    Hi everybody,

    I am trying to make an adress book where I can store a persons name and his phone numbers.
    I am using the Microsoft Visual C++ 2005 IDE.
    There are 3 kinds of numbers :
    Code:
    enum number_type {MOBILE = 0, PRIVATE = 1, WORK = 2};
    A contact record is represented by:
    Code:
    struct contact_record {
    	char* contact_name;
    	char*** numbers;
    	int* numbers_size;
    };
    
    typedef struct contact_record contact;
    contact_name stores the name of the person, numbers is a 2D array of numbers(=strings)(1 row for each type of number), and numbers_size keeps the size of the numbers of each sort.

    The tree relevant functions (where the error might occur) are:
    Code:
    contact init_record(char* contact_name);
    void add_number(contact* c, enum number_type numbertype, char* number);
    void delete_contact(contact* c);
    The implementation of those functions is:
    Code:
    contact init_record(char* contact_name){
    	int i,j;
    	contact* c;
    	char*** numbers;
    	int* numbersize;
    
    	c = (contact*)malloc(sizeof(contact));
    	//contactname = malloc(20*sizeof(char));
    	numbers = malloc(sizeof(char**));
    	for(i=0; i < 3; i++) {
    		numbers[i] = malloc(sizeof(char*));
    		for(j=0; j < 1; j++)
    			numbers[i][j] = malloc(15*sizeof(char));
    	}
    	numbersize = calloc(3,sizeof(int));
    
    	c->contact_name = contact_name;
    	c->numbers = numbers;
    	c->numbers_size = numbersize;
    
    	return *c;
    }
    
    void add_number(contact* c, enum number_type numbertype, char* number){
    	switch(numbertype) {
    		case MOBILE:
    			c->numbers_size[0]++;
    			c->numbers[0] = realloc(c->numbers[0], c->numbers_size[0]*sizeof(char*));
    			c->numbers[0][c->numbers_size[0]-1] = number;
    			break;
    		// same for PRIVATE and HOME
    	}
    }
    
    void delete_contact(contact* c){
    	int i;
    	free(c->numbers_size);
    	for(i=0; i < 3; i++) {
    		free(c->numbers[i]);
    	}
    	free(c->numbers);
    	free(c->contact_name);
    	free(c);
    }
    The error occurs when reaching the line:
    Code:
    free(c->numbers);
    Is there anyone who might have an idea wath I'm doing wrong?
    Thanks in advance,

    Steven

  2. #2
    ZuK
    ZuK is offline
    Registered User
    Join Date
    Aug 2005
    Location
    Austria
    Posts
    1,990
    Code:
    	numbers = malloc(sizeof(char**));
    	for(i=0; i < 3; i++) {
    		numbers[i] = malloc(sizeof(char*));
    For numbers you allocate only space for a single pointer.
    Kurt

  3. #3
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,452
    > for(j=0; j < 1; j++)
    This is only 1 element long, so what's the point of this dimension of the array?

    It seems to me that
    char *numbers[3];
    is a lot simpler, where the subscript is your enum.

    If someone doesn't have a work number say, then the corresponding entry in the numbers array is NULL.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  4. #4
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,296
    Alarm bells started ringing when I saw this:
    Code:
    char*** numbers;
    This looks suspiciously like it should be a strcpy:
    Code:
    		c->numbers[0][c->numbers_size[0]-1] = number;
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  5. #5
    Registered User
    Join Date
    Jan 2008
    Posts
    7
    Quote Originally Posted by Salem View Post
    > for(j=0; j < 1; j++)
    This is only 1 element long, so what's the point of this dimension of the array?
    I start with a space for one number of each type, and every time I add a number I reallocate one space extra. (Saving my memory)

    Thanks for your help, but my problem isn't solved yet.
    the strcpy still gives me the same error.

    Someone who might know what the problem is?
    Thanks in advance,

    Steven

  6. #6
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by svdrjeug View Post
    I start with a space for one number of each type, and every time I add a number I reallocate one space extra. (Saving my memory)

    Thanks for your help, but my problem isn't solved yet.
    the strcpy still gives me the same error.

    Someone who might know what the problem is?
    Thanks in advance,

    Steven
    Is this for some sort of minimal embedded system? If not, then I would strongly suggest that small memory savings like that are overshadowed by the memory tracking data that the C runtime uses when you allocate memory. On a Windows system with MS Visual Studio, for example, one allocation takes up 64 bytes minimum.

    Even on an embedded system with small amounts of memory, it's highly likely that a memory allocation takes up 8-16 bytes for "maintenance", on top of what YOU actually ask for.

    Also, are you sure the c->numbers[0..2] are set to NULL if you haven't allocated all of them - I haven't checked the code thoroughly, but it's an obvious place where things may go wrong:
    Code:
     	
            for(i=0; i < 3; i++) {
    		free(c->numbers[i]);
    	}

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  7. #7
    Registered User
    Join Date
    Jan 2008
    Posts
    7
    It is not for en embedded system that I am writing this. I justed want to exercise the realloc function.

    The c->numbers[0..2] are all allocated in this piece of code
    Code:
    numbers = malloc(sizeof(char**));
    	for(i=0; i < 3; i++) {
    		numbers[i] = malloc(sizeof(char*));
    		for(j=0; j < 1; j++)
    			numbers[i][j] = malloc(15*sizeof(char));
    	}
    I think this piece is correct.
    The tree rows are correctly freed but it's in this line
    Code:
    free(c->numbers);
    that it goes wrong.
    This is strange, because the are only 3 rows in the table, and they are all 3 freed correctly.
    I don't understand why there is an error at freeing the whole array then.

    Steven

  8. #8
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    So is c->numbers the same value when you set it and when you try to free it?
    What exactly is the error-message (or mis-behaviour) you get when freeing it?

    If you use the debugger in Visual studio, you can probably find out the answer to the first question. If it changes, then you need to find out where this happens...

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  9. #9
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,452
    If you start with

    Code:
    arr = malloc ( sizeof *arr * nElems );
    Then to expand, it's
    Code:
    void *temp = realloc( arr, sizeof *arr * (nElems + nExtra) );
    if ( temp != NULL ) {
      arr = temp;
      nElems += nExtra;
    } else {
      // old arr is still valid.  At the very least, you'll need to do
      // free(arr)
      // at some point, perhaps after giving the user some options.
    }
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  10. #10
    Registered User
    Join Date
    Jan 2008
    Posts
    7
    Quote Originally Posted by matsp View Post
    So is c->numbers the same value when you set it and when you try to free it?
    What exactly is the error-message (or mis-behaviour) you get when freeing it?

    If you use the debugger in Visual studio, you can probably find out the answer to the first question. If it changes, then you need to find out where this happens...

    --
    Mats
    I have debugged my program and the address in the begin and at the end of the c->numbers are the same. So that's not the problem, I assume.

    When I debug, and I try to free the array, I get a Visual C++ window with the text
    Code:
    Windows has triggered a breakpoint in adressbook.exe.
    
    This may be due to a corruption of the heap, and indicates a bug in adressbook.exe or any of the DLLs it has loaded.
    
    The output window may have more diagnostic information
    The output window doesn't show anything extra.

  11. #11
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    That, usually, indicates that you have gone out of bounds within one of your memory allocations, e.g. allocate space for 10 elements and write 11 or more.

    Why don't you post your current code?

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  12. #12
    Registered User
    Join Date
    Jan 2008
    Posts
    7
    Ok, here is my full implementation. It is almost exactly the same as the one I posted in the beginning of this topic.
    I have tried to make some changes as suggested, but they didn't solve the problem, so I've put my original code back.

    Code:
    #include <stdlib.h>
    #include <stdio.h>
    
    #include "contact_record.h"
    
    contact init_record(char* contact_name){
    	int i,j;
    	contact* c;
    	char*** numbers;
    	int* numbersize;
    
    	c = (contact*)malloc(sizeof(contact));
    	//contactname = malloc(20*sizeof(char));
    	numbers = malloc(sizeof(char**));
    	for(i=0; i < 3; i++) {
    		numbers[i] = malloc(sizeof(char*));
    		for(j=0; j < 1; j++)
    			numbers[i][j] = malloc(15*sizeof(char));
    	}
    	numbersize = calloc(3,sizeof(int));
    
    	c->contact_name = contact_name;
    	c->numbers = numbers;
    	c->numbers_size = numbersize;
    
    	return *c;
    }
    
    void add_number(contact* c, enum number_type numbertype, char* number){
    	switch(numbertype) {
    		case MOBILE:
    			c->numbers_size[0]++;
    			c->numbers[0] = realloc(c->numbers[0], c->numbers_size[0]*sizeof(char*));
    			c->numbers[0][c->numbers_size[0]-1] = number;
    			break;
    		case PRIVATE:
    			c->numbers_size[1]++;
    			c->numbers[1] = realloc(c->numbers[1], c->numbers_size[1]*sizeof(char*));
    			c->numbers[1][c->numbers_size[1]-1] = number;
    			break;
    		case WORK:
    			c->numbers_size[2]++;
    			c->numbers[2] = realloc(c->numbers[2], c->numbers_size[2]*sizeof(char*));
    			c->numbers[2][c->numbers_size[2]-1] = number;
    			break;
    	}
    }
    
    void delete_contact(contact* c){
    	int i;
    	free(c->numbers_size);
    	for(i=0; i < 3; i++) {
    		free(c->numbers[i]);
    	}
    	free(c->numbers);
    	free(c->contact_name);
    	free(c);
    }
    Last edited by svdrjeug; 01-02-2008 at 06:25 AM.

  13. #13
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Correct me if I'm wrong, but:
    Code:
    			c->numbers[0][c->numbers_size[0]-1] = number;
    changes the pointer allocated here:
    Code:
    		for(j=0; j < 1; j++)
    			numbers[i][j] = malloc(15*sizeof(char));

    Where does "number" in the first code snippet come from?

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  14. #14
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,452
    > I start with a space for one number of each type, and every time I add a number I reallocate
    > one space extra. (Saving my memory)
    From what I can see of your usage so far, a person can have more than one mobile number (for example).
    Is this correct?
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  15. #15
    Registered User
    Join Date
    Jan 2008
    Posts
    7
    Quote Originally Posted by matsp View Post
    Correct me if I'm wrong, but:
    Code:
    			c->numbers[0][c->numbers_size[0]-1] = number;
    changes the pointer allocated here:
    Code:
    		for(j=0; j < 1; j++)
    			numbers[i][j] = malloc(15*sizeof(char));

    Where does "number" in the first code snippet come from?

    --
    Mats
    "number" is the string representation of a phone number. It is given as a parameter of the add_number method.

    and I think you right,
    Code:
    c->numbers[0][c->numbers_size[0]-1] = number
    gets a new pointer here, but does this matter when freeing the memory, because it is overwritten?

    Quote Originally Posted by Salem View Post
    From what I can see of your usage so far, a person can have more than one mobile number (for example).
    Is this correct?
    You are right.
    That's an extra feature

Page 1 of 2 12 LastLast
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. To find the memory leaks without using any tools
    By asadullah in forum C Programming
    Replies: 2
    Last Post: 05-12-2008, 07:54 AM
  2. Assignment Operator, Memory and Scope
    By SevenThunders in forum C++ Programming
    Replies: 47
    Last Post: 03-31-2008, 06:22 AM
  3. Dynamically allocated memory
    By ^xor in forum Linux Programming
    Replies: 9
    Last Post: 06-28-2005, 11:42 AM
  4. pointers
    By InvariantLoop in forum C Programming
    Replies: 13
    Last Post: 02-04-2005, 08:32 AM
  5. Allcoating memory and not freeing it
    By lambs4 in forum C Programming
    Replies: 1
    Last Post: 09-05-2001, 11:32 AM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21