Thread: crash using realloc

  1. #1
    Registered User
    Join Date
    Feb 2005
    Posts
    61

    crash using realloc

    I'm trying to writ my own small string library, and I've encountered a problem which I can't solve. When trying to implement a strcat function, I seem to hav memory problems. I think it's because of the use of realloc, but I'm not sure.
    Here is the code :

    Code:
    int main(){
    	char *t = malloc(sizeof(char) * 6);
    	char *l = malloc(sizeof(char) * 6);
    
    	H_strCopy(t, "hallo");
    	H_strCopy(l, t);
    	H_strInvert(l);
    
    	H_strCat(t, l);
    	H_strCat(t, l);
    	H_strCat(t, l);
    
    	H_strPrint(t);
    
    	free(l);
    	free(t);
    
    	return 0;
    }
    Code:
    void H_strCat (char *str1, char *str2){
    	char *temp;
    	int len1, len2, newlen, i;
    
    	len1 = H_strLength(str1);
    	len2 = H_strLength(str2);
    	newlen = len1 + len2 + 1;
    
    	temp = malloc(sizeof(char) * (len1+1));
    	H_strCopy(temp, str1);
    
    	str1 = realloc(str1, sizeof(char) * newlen);
    	
    	for (i=0; i<len1; i++) str1[i] = temp[i];
    	for (i=0; i<=len2; i++) str1[len1 + i] = str2[i];
    
    	free(temp);
    }
    When I call H_strCat() 2 times or less in main(), the program doesn't crash. It does however when I call it 3 times or more. Does anyone have a clue ?

    btw : The other functions I use (like H_strCopy() ) are probably not the problem, because I don't use any malloc, realloc, free, or any other memory management function in them.

  2. #2
    Registered User Micko's Avatar
    Join Date
    Nov 2003
    Posts
    715
    Well it smells like undefined behavior, I'd checked and tested other function as well to make sure but pay attention:
    1. what value will have len1 after this line len1 = H_strLength(str1); str1 is actually what?
    2. You complicate too much when playing with realloc. Please see this:
    Code:
        char *as; 
        char mi[100];
        int i;
        as = malloc(100);  
        strcpy(as, "Asim");
        strcpy(mi, " Micko");
        size_t len = strlen(as);
        size_t len2 = strlen(mi);
        as = realloc(as, len + len2 + 1);
        for (i = 0; i < len2; i++)
            as[i + len] = mi[i];
        as[len+len2] = '\0';/* I simply like this way*/
    because the realloc function changes the size of the object
    pointed to by ptr to the size specified by size. The
    contents of the object shall be unchanged up to the lesser
    of the new and old sizes.
    3. sizeof(char) is ALWAYS 1 according to the Standard
    4. You're trying to con. str1 and str2, but there is a small problem, str1 and str2 are local to function, and because function returns nothing no actual changes are possible (they are, with using double pointer...) in main
    4. in C you should use int main(void), see FAQ for details...
    Gotta love the "please fix this for me, but I'm not going to tell you which functions we're allowed to use" posts.
    It's like teaching people to walk by first breaking their legs - muppet teachers! - Salem

  3. #3
    ATH0 quzah's Avatar
    Join Date
    Oct 2001
    Posts
    14,826
    Actually, the problem is that you're passing a pointer to a function, and then inside the function, trying to change what the pointer points to. You can't do that. If you need to change what a pointer points to, you have to pass a pointer to that pointer. Thus, it should be:
    Code:
    void H_strCat (char **str1, char *str2){
        ...stuff...
    }
    For those disbelievers out there, try it yourself:
    Code:
    #include <stdio.h>
    #include <string.h>
    #include <stdlib.h>
    
    void re( char *s )
    {
        s = realloc( s,strlen( s ) * 2 );
        strcpy( s, "Foo!!" );
    
        printf("s is %s\n", s );
    }
    
    int main( void )
    {
        char *s;
    
        s = malloc( strlen( "Hello World!" ) + 1 );
        strcpy( s, "Hello World!" );
    
        printf("s is %s\n", s );
    
        re( s );
    
        printf("s is %s\n", s );
    
        return 0;
    }
    
    /*
    
    s is Hello World!
    s is Foo!!
    s is 
    
    */
    Additionally, you're both using realloc incorrectly. You shouldn't ever directly assign the pointer returned from realloc to the memory you're allocating, in case it fails. Yes, I did it in the above example, but I was just illustrating what your code does.

    This is the correct way to realloc:
    Code:
    void re( char *s )
    {
        char *ptr = realloc( s,strlen( s ) * 2 );
        strcpy( s, "Foo!!" );
    
        printf("s is %s\n", s );
    
        s = ptr;
    }
    And looking at the last line, you should now be able to see what I was talking about earlier, and why this fails. It fails because you cannot assign a pointer to another inside a function and have it update the value outside the function.

    Rather, you'd have to pass a pointer to a pointer, like so:
    Code:
    void re2( char **s )
    {
        char *ptr = realloc( *s, strlen( *s ) * 2 );
        strcpy( ptr, "Foo!!" );
    
        *s = ptr;
        printf("*s is %s\n", *s );
    }
    Quzah.
    Hope is the first step on the road to disappointment.

  4. #4
    Registered User
    Join Date
    Feb 2005
    Posts
    61
    Thanks for the very useful replies. I've rewritten the function, and it seems to work now (even for more than 1000 strCat's). Just to be sure : this is the rewritten function. Please tell me if I still did anything unsafe or wrong.
    Code:
    void H_strCat (char **str1, char *str2){
    	char *temp, *ptr;
    	int len1, len2, len, i;
    
    	len1 = H_strLength(*str1);
    	len2 = H_strLength(str2);
    	len = len1 + len2 + 1;
    
    	ptr = realloc(*str1, sizeof(char) * len);
    	
    	for (i=0; i<len1; i++) (ptr)[i] = (*str1)[i];
    	for (i=0; i<=len2; i++)	(ptr)[len1 + i] = str2[i];
    
    	*str1 = ptr;
    }
    I have two other questions :
    1) Where can I read some more about pointers to pointers.
    2) Is it possible to make this function in such a way that I only have to pass a single pointer instead of a pointer to a pointer? Just for uniformity.

  5. #5
    Registered User Micko's Avatar
    Join Date
    Nov 2003
    Posts
    715
    Quote Originally Posted by hannibar
    1) Where can I read some more about pointers to pointers.
    2) Is it possible to make this function in such a way that I only have to pass a single pointer instead of a pointer to a pointer? Just for uniformity.
    Three's plenty on the Internet. I'm sure you will find what you need. Pointer to a pointer is not scary thing as it may seems at the first glance.
    The traditional method to allow a function to change its caller's memory is to
    pass a pointer to the caller's memory instead of a copy. So in C, to change an int in the
    caller, pass a int* instead. To change a struct fraction, pass a struct
    fraction* intead. To change an X, pass an X*. So in this case, the value we want to
    change is struct node*, so we pass a struct node** instead. The two stars
    (**) are a little scary, but really it's just a straight application of the rule. It just happens
    that the value we want to change already has one star (*), so the parameter to change it
    has two (**)

    So if you want to change pointer variable you need to pass address of that variable, so you use pointer to pointer.
    You can write function which takes pointer but then also returns pointer...

    - Micko
    Last edited by Micko; 04-13-2005 at 02:45 AM.
    Gotta love the "please fix this for me, but I'm not going to tell you which functions we're allowed to use" posts.
    It's like teaching people to walk by first breaking their legs - muppet teachers! - Salem

  6. #6
    Registered User
    Join Date
    Apr 2004
    Posts
    173
    Another good way to think about pointers to pointers is a level-based system.
    So we had the following (assuming the values are properly initialised beforehand):

    char** ppc;
    char* pc;
    char c;

    "Level 1" - Standard char
    **ppc, *pc, c

    "Level 2" - Pointer to char
    *ppc, pc, &c

    "Level 3" - Pointer to Pointer to char
    ppc, &pc

    This is quite a basic view on things but gives you an extra perspective to look at.

    Btw, Quzah what compiler do you use? 'Cos I'm using GCC 3.3.4 and the following program still works "correctly":

    Code:
    #include <stdio.h>
    #include <string.h>
    #include <stdlib.h>
    
    void re( char *s )
    {
            char* ptr = realloc(s,strlen(s) * 2);
            strcpy( s, "Foo!!" );
    
            printf("s is %s\n", s );
            s = ptr;
    }
    
    void re2( char *s)
    {
            char* ptr = realloc(s,strlen(s) * 2);
            strcpy(s, "Bar!!");
    
            printf("s is %s\n",s);
            s = ptr;
    }
    
    int main( void )
    {
        char *s;
    
        s = malloc( strlen( "Hello World!" ) + 1 );
        strcpy( s, "Hello World!" );
    
        printf("s is %s\n", s );
    
        re( s );
    
        printf("s is %s\n", s );
    
        re2 (s);
    
        printf("s is %s\n", s);
    
        return 0;
    }
    The output I get is:
    Code:
    bash-3.00# ./realloc1
    s is Hello World!
    s is Foo!!
    s is Foo!!
    s is Bar!!
    s is Bar!!
    It's a thing thats been bugging me for quite a while - since I usually use pointer to pointers to modify the pointer but everyone else just uses a pointer argument and yields the same results - in lets say for example: in a linked list insertion function where you have to modify the "root" node pointer. I'm thinking this is more of a GCC thing rather than the C standard but any confirmations would be good
    Last edited by 0rion; 04-13-2005 at 07:38 AM.

  7. #7
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,656
    Your code is plenty wrong, remember working correctly != a correct program.
    There is no obligation for a system to throw a segfault each and every time you mess up.

    Here's your first mistake
    char* ptr = realloc(s,strlen(s) * 2);
    strcpy( s, "Foo!!" );
    realloc may have moved memory (s != ptr), so it is no longer correct to dereference memory via s

    > s = ptr;
    Doesn't change the pointer value used in the caller.
    If you run with enough warnings (or lint), you'll get an "assignment is never used" type message.
    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.

  8. #8
    Registered User
    Join Date
    Apr 2004
    Posts
    173
    Nah what I was trying to say is that it does infact change the value of s in the caller function as the output outputted correctly, I was just trying to see if this was a GCC thing that changes it to the "intended" object code.

  9. #9
    Registered User
    Join Date
    Jun 2004
    Posts
    201
    Quote Originally Posted by quzah
    Code:
    void re2( char **s )
    {
        char *ptr = realloc( *s, strlen( *s ) * 2 );
        strcpy( ptr, "Foo!!" );
    
        *s = ptr;
        printf("*s is %s\n", *s );
    }
    Not testing the return value from realloc is just as wrong as assiging the return value to the same pointer as you're passing in.

  10. #10
    SleepWalker tjohnsson's Avatar
    Join Date
    Apr 2004
    Posts
    70
    Quote Originally Posted by hannibar
    ...
    2) Is it possible to make this function in such a way that I only have to pass a single pointer instead of a pointer to a pointer? Just for uniformity.
    Sure, it's possible, what isn't with C.
    this is not so good practice and i'm
    quite sure, you find usage of pointer to pointers
    much more efficient than this one.

    Code:
     
    void
    my_alloc (char *ptr) {
        *(char **)ptr = malloc (123);
    }
    
    void 
    my_realloc (char *ptr) {
        *(char **)ptr = realloc (*(char **)ptr, 321);
    }
    
    int 
    main(void) {
        char *str = "string";
        char *ptr = null;
        
        my_alloc ((char *)&ptr);
        my_realloc ((char *)&ptr);    
        memset (ptr, 0, 321);
        
        strcpy (ptr, str);
        puts (ptr);
        
        free (ptr);
        
        return 0;    
    }
    -- Add Your Signature Here --

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. did i understood right this explantion of realloc..
    By transgalactic2 in forum C Programming
    Replies: 3
    Last Post: 10-24-2008, 07:26 AM
  2. Can not debug a crash
    By hannibar in forum Windows Programming
    Replies: 2
    Last Post: 06-30-2007, 10:02 AM
  3. using realloc
    By bobthebullet990 in forum C Programming
    Replies: 14
    Last Post: 12-06-2005, 05:00 PM
  4. FYI: asctime(gmtime(&mytime)) = crash!
    By anonytmouse in forum C Programming
    Replies: 2
    Last Post: 09-29-2003, 02:24 AM
  5. segfault on realloc
    By ziel in forum C Programming
    Replies: 5
    Last Post: 03-16-2003, 04:40 PM