#1
  1. No Profile Picture
    Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    May 2009
    Posts
    30
    Rep Power
    6

    Why do I get this error message while freeing all nodes in Doubly Linked List


    Hi, I have a DLL and a method to free all nodes of DLL. If I execute this method after creating DLL or after making some changes to it like inserting and deleting or changing few strings, it runs fine. But after playing with all these for a while and trying to execute the same (freeing all the nodes of DLL) gives me the following message:

    *** glibc detected *** ./ed: free(): invalid next size (fast): 0x09872068 ***
    ======= Backtrace: =========
    /lib/i686/cmov/libc.so.6[0xb7ea7624]
    /lib/i686/cmov/libc.so.6(cfree+0x96)[0xb7ea9826]
    ./ed[0x8048f8c]
    ./ed[0x8048e31]
    /lib/i686/cmov/libc.so.6(__libc_start_main+0xe5)[0xb7e4f455]
    ./ed[0x8048661]
    ======= Memory map: ========
    08048000-0804a000 r-xp 00000000 08:02 500789 /home/jay/***2/ed
    0804a000-0804b000 rw-p 00001000 08:02 500789 /home/jay/***2/ed
    09851000-09893000 rw-p 09851000 00:00 0 [heap]
    b7d00000-b7d21000 rw-p b7d00000 00:00 0
    b7d21000-b7e00000 ---p b7d21000 00:00 0
    b7e1e000-b7e2a000 r-xp 00000000 08:02 524330 /lib/libgcc_s.so.1
    b7e2a000-b7e2b000 rw-p 0000b000 08:02 524330 /lib/libgcc_s.so.1
    b7e38000-b7e39000 rw-p b7e38000 00:00 0
    b7e39000-b7f8e000 r-xp 00000000 08:02 532490 /lib/i686/cmov/libc-2.7.so
    b7f8e000-b7f8f000 r--p 00155000 08:02 532490 /lib/i686/cmov/libc-2.7.so
    b7f8f000-b7f91000 rw-p 00156000 08:02 532490 /lib/i686/cmov/libc-2.7.so
    b7f91000-b7f94000 rw-p b7f91000 00:00 0
    b7f9f000-b7fa3000 rw-p b7f9f000 00:00 0
    b7fa3000-b7fa4000 r-xp b7fa3000 00:00 0 [vdso]
    b7fa4000-b7fbe000 r-xp 00000000 08:02 524623 /lib/ld-2.7.so
    b7fbe000-b7fc0000 rw-p 0001a000 08:02 524623 /lib/ld-2.7.so
    bfbaa000-bfbbf000 rw-p bffeb000 00:00 0 [stack]
    Aborted
    and here's my implementation of the method:
    PHP Code:
    void expunge(DLListPtr *sPtrHeaderPtr *hPtrint *flag) {
        
    DLListPtr tmp;
        while (*
    sPtr) {
            
    tmp = (*sPtr)->nextPtr;
            
    free(*sPtr);
            *
    sPtr tmp;
        }
        *
    hPtr NULL;
        *
    flag 0;

    where, HeaderPtr is a single node which has pointer to the head, tail, current position of Doubly Linked List and pos as an integer which holds the position of each node. Thank you.
  2. #2
  3. Contributing User
    Devshed Supreme Being (6500+ posts)

    Join Date
    Jan 2003
    Location
    USA
    Posts
    7,255
    Rep Power
    2222
    Wild guess: the pointer you're trying to free doesn't have anything malloc'd to it?

    Sorry, but since I've never tried to free an unallocated pointer, I don't know what to expect. My usual practice is to set all un-malloc'd pointers to a known value, NULL, and then only attempt to free non-NULL pointers.
  4. #3
  5. No Profile Picture
    Contributing User
    Devshed Novice (500 - 999 posts)

    Join Date
    Oct 2007
    Posts
    921
    Rep Power
    537
    Originally Posted by dwise1_aol
    Sorry, but since I've never tried to free an unallocated pointer, I don't know what to expect.
    If a non-NULL pointer wasn't returned by malloc()/realloc()/calloc() then providing it as an argument to free() yields undefined behaviour according to both C and C++ standards. Anything can therefore happen as a result.
    Right 98% of the time, and don't care about the other 3%.

    “It has been said that the great scientific disciplines are examples of giants standing on the shoulders of other giants. It has also been said that the software industry is an example of midgets standing on the toes of other midgets.” (Alan Cooper)
  6. #4
  7. No Profile Picture
    Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    May 2009
    Posts
    30
    Rep Power
    6
    Thanks guys for the replies. One quick one being related to what you guys are suggesting: By what I have understood:
    DLListPtr startPtr = NULL;
    This initializing to NULL, allocates address for startPtr in the memory right? and:
    startPtr = malloc(sizeof(DLList));
    This allocates the size of DLList for startPtr in the memory?
    Now, when I call free(startPtr) will it completely remove startPtr, i.e. even the address allocated for it or just the size allocated by malloc? If it completely removes startPtr then before doing anything with startPtr, I should call startPtr = NULL right so that again some address is allocated for it? And if I have some pointer in startPtr pointing to some data (e.g. startPtr->head = "some value") and happens if I do free(startPtr) or startPtr = NULL. I would appreciate if any one could explain this in plain language. Thank you
  8. #5
  9. Contributed User
    Devshed Specialist (4000 - 4499 posts)

    Join Date
    Jun 2005
    Posts
    4,417
    Rep Power
    1871
    If you have
    sometype *p = NULL;
    p = malloc( sizeof *p * howManyAreNeeded );


    Then at some point later on, you must do
    free( p ); p = NULL;

    If you want, you can add the blue code just to make it clear that if a pointer it unused, it is always NULL.

    > And if I have some pointer in startPtr pointing to some data (e.g. startPtr->head = "some value")
    > and happens if I do free(startPtr) or startPtr = NULL
    Invariably, this leaks memory.
    If you have a pointer to some kind of nested data structure like a linked list, then you need to free the things being pointed to before freeing the top-level thing.

    Your approach in post#1 of using a temp to walk down the list freeing nodes is good.
    But we need to see the actual definitions of the types, and how you call that function to make some sense of why it might not work.
    It could work, given the right kind of environment.
    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
  10. #6
  11. No Profile Picture
    Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    May 2009
    Posts
    30
    Rep Power
    6
    Originally Posted by salem
    >Invariably, this leaks memory.
    If you have a pointer to some kind of nested data structure like a linked list, then you need to free the things being pointed to before freeing the top-level thing.
    Thanks for the reply salem. Ok so, there I'm freeing all the nodes and the nodes its pointing to but what about the "char *string" as startPtr points to string as "startPtr->string = "xyz";" Here's the definition of structure for DLL:
    PHP Code:
    struct dll {
      
    char *string;
      
    struct dll *next;
      
    struct dll *prev;
    }
    typedef struct dll DLList;
    typedef DLList *DLListPtr
    But we need to see the actual definitions of the types, and how you call that function to make some sense of why it might not work.
    It could work, given the right kind of environment.
    In the main I have done "DLListPtr startPtr = NULL;" and using switch statement I have called the function as "expunge(&startPtr, &hPtr);" where &hPtr is also created just like dll to hold pointers to DLList for head, tail and current position of DLList. It also has int to hold line number of current position of DLList. IT is hard to say when coz it doesn't give that message often. And as far as I know and double checked as well, all my create and insert calls malloc to create nodes.
    Also about delete, when I delete the last node in the list I have called free and when I print out the list instead of saying "EMPTY" it prints "#0" as before calling print method I have checked if the startPtr is NULL, which means after free(startPtr) I should call startPtr=NULL as well?. Below is my implementation:

    PHP Code:
    case 'p':
     if (!
    isEmpty(startPtr))
        
    show(startPtr);
     else
        
    printf("EMPTY\n");
    break;
    ....
    void show(DLListPtr sPtr){
      while (
    sPtr != NULL) {
         
    printf("%s\n"sPtr->string);
         
    sPtr sPtr->next;
      }

    And in delete method:
    PHP Code:
    void delete(DLListPtr *sPtrHeaderPtr *hPtrint pos){
      
    DLListPtr curPtr = *sPtr;
      
    //incremented the curPtr until pos
      
    if (curPtr == *sPtr){ //i.e. pos is 0.
         
    if (curPtr->next)
            
    delete until curPtr->next is true
         
    else{ //i.e. this is the last node of DLList
            
    free(*sPtr);
            
    //*sPtr = NULL; //should i call this as well
            
    free(*hPtr);
            
    //*hPtr = NULL; //should i call this as well
         
    }    
      }    

    So, should I call "*sPtr=NULL" as well coz if I just call "free(*sPtr)" print method still prints blank instead of printing "EMPTY".
    Sorry, it went long and hope it's not messy. Thanks for your patience to go through all my problem and question. I really appreciate it.
  12. #7
  13. Devshed God 1st Plane (5500 - 5999 posts)

    Join Date
    Jun 2005
    Posts
    5,964
    Rep Power
    4852
    What you do with "char *string;" depends. This code,
    Code:
     struct dll {
      char *string;
      struct dll *next;
      struct dll *prev;
    }
    typedef struct dll DLList;
    typedef DLList *DLListPtr;
    defines a struct and makes a couple of types. The memory used for that struct and the memory used for that struct's members depend entirely upon how you instantiate it.

    Anything instantiated on the heap needs to be freed. Locals, statics, and globals do not.

    The code that you show does not include those operations, so anything concerning that would just be a guess.

    What is your understanding regarding heap, static, and local memory?
    Write no code whose complexity leaves you wondering what the hell you did.
    Politically Incorrect DaWei on Pointers Grumpy on Exceptions
  14. #8
  15. No Profile Picture
    Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    May 2009
    Posts
    30
    Rep Power
    6
    Originally Posted by sizablegrin
    Anything instantiated on the heap needs to be freed. Locals, statics, and globals do not.

    The code that you show does not include those operations, so anything concerning that would just be a guess.

    What is your understanding regarding heap, static, and local memory?
    Ya, "char *string;" is instantiated to use the heap size using malloc as well. By the way, I have also created local variables as "DLListPtr curPtr;" which is instantiated as "curPtr = *startPtr;", so I don't need to free(curPtr)?
    Also as I'm print "EMPTY" message only if the startPtr is pointing to null, I should do startPtr = NULL as well right after free(startPtr) as this leaves the startPtr uninitialized state pointing nowhere right? Thanks.
    Also, I already malloc'd startPtr->string before while initializing it some value(string). Now, as I do some editing to this string, it might go bigger or smaller, i.e. lot more characters might be added or deleted from this string so in the case when more characters are added to this string the before allocated space with malloc might not be sufficient so I'm mallocating again for this startPtr->string. So, at this point, should I first free(startPtr->string) and again malloc for startPtr->string? Thanks
    Also, why the following free(..) gives me error?
    PHP Code:
    case 'c':
        if (!
    isEmpty(startPtr)) {
            
    getInput("Text to replace: "strtorepNULL);
            
    getInput("Replacement text: "repstrNULL);
            
    repstr[strlen(repstr)-1] = '\0';
            
    strtorep[strlen(strtorep)-1] = '\0';
          
    char *newstring = (char*)malloc(strlen(curPtr->string)+strlen(repstr));
            
    newstring replace(curPtr->string, &hPtrstrtoreprepstr);
            if (
    newstring != NULL) {
                
    curPtr->string = (char*)malloc(strlen(newstring));
                
    strcpy(curPtr->stringnewstring);
                
    free(newstring); //Giving error
                
    printf("#%d %s\n"atoi(&input[2]), curPtr->string);
            }
            else
                
    printf("String not found.\n");
        }
            else
                 
    printf("EMPTY/n");
    break;

    charreplace(char *sPtrHeaderPtr *hPtrchar strtorep[], char repstr[]) {
        
    char *strPtr;
        static 
    char tmp[MAX_LENGTH];
        if ((
    strPtr strstr(sPtrstrtorep)) != NULL) {
            
    strncpy(tmpsPtrstrPtr-sPtr);
            
    tmp[strPtr-sPtr] = '\0';
            
    sprintf(tmp+(strPtr-sPtr), "%s%s"repstrstrPtr+strlen(strtorep));
            return 
    tmp;
        }
        return 
    NULL;

    Does newstring have to exist for curPtr->string to point to? I thought strcpy(curPtr->string, newstring) copies the value of newstring to curPtr->string and I no longer require newstring but I think I'm wrong.
  16. #9
  17. Contributed User
    Devshed Specialist (4000 - 4499 posts)

    Join Date
    Jun 2005
    Posts
    4,417
    Rep Power
    1871
    > char *newstring = (char*)malloc(strlen(curPtr->string)+strlen(repstr));
    > newstring = replace(curPtr->string, &hPtr, strtorep, repstr);
    This is a memory leak.
    The result of the malloc is trashed by the return result.
    Since the return result is static, there's no need for this malloc, nor your attempt to free it.

    > curPtr->string = (char*)malloc(strlen(newstring));
    > strcpy(curPtr->string, newstring);
    You're 1 byte short on the memory.
    strlen() returns the number of chars, but a string ALSO needs space for the \0 as well.

    > return tmp;
    You're returning a pointer to a static memory location.
    If you try and free this (as you do), you will die.

    If you didn't malloc it, DON'T free it.
    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
  18. #10
  19. No Profile Picture
    Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    May 2009
    Posts
    30
    Rep Power
    6
    Thanks for the reply. So, I can directly do "char *newstring = replace(......);" without mallocating for newstring as "replace(...)" is returning static array? Also, now what about the curPtr->string. As I've already malloc'd it before while making node, I want to malloc it again as I've altered the string it holds. So, should I first free(curPtr->string) and then malloc it with the new size of the string or I can directly malloc it and it will take this new allocation? Thank you.
  20. #11
  21. Devshed God 1st Plane (5500 - 5999 posts)

    Join Date
    Jun 2005
    Posts
    5,964
    Rep Power
    4852
    What you need to do is stop flailing about and shooting from the hip. Think about what you need, get, have, and dispose of.

    Mallocing for memory is borrowing from a bank. Ultimately, you must return what you borrowed.

    The pointer returned by malloc (if any) has two functions: it serves as a pointer to an area of memory which must be later released, and it serves as a pointer to the contents of that area, which you will use as storage.

    If you have such a pointer and you assign a new value to it, you have lost the original value - you won't have it to pass to "free()" at some later time.

    If you intend to alter its value you need to free the memory pointed to by its current value, first, then alter it. Alternatively, you could store it in a second list, change the value, and later go through the list, freeing the memory pointed to by each of its elements.

    I strongly recommend that you develop knowledge regarding system memory (a collection of hardware storage elements), and how it is partitioned and dealt with by the system and its OS (executive and memory manager). The complexity of those items varies, but they're present in some form, however rudimentary. (Heaps (memory pools, not tree structures) are not always implemented by the system; they may be implemented by a language or not at all.)
    Write no code whose complexity leaves you wondering what the hell you did.
    Politically Incorrect DaWei on Pointers Grumpy on Exceptions
  22. #12
  23. No Profile Picture
    Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    May 2009
    Posts
    30
    Rep Power
    6
    Thanks for the recommendation, ya I'm going through as much as I can.

    Comments on this post

    • sizablegrin disagrees : But you're coming here confused. That's alright, but you're expecting us to fix it for you without an expenditure of energy on your part.
  24. #13
  25. Commie Mutant Traitor
    Devshed Intermediate (1500 - 1999 posts)

    Join Date
    Jun 2004
    Location
    Alpharetta, GA
    Posts
    1,806
    Rep Power
    1570
    One additional recommendation is that you always check the pointer returned by malloc() to make sure it's valid. If there is some reason that memory cannot be allocated (rare in modern systems, as you usually have room to spare, but it can happen if you're running several leaky programs at once), malloc() returns NULL. If you don't check this, you could end up with a segfault somewhere in the middle of code that had seemed to worked right, weeks or months down the line. Thus, any malloc() call ought to be followed by something like this:
    Code:
        if (allocated_node == NULL)
        {
            /* handle the error case here */
        }
    Skipping or forgetting to check for error states is all too common in C programming, especially older code (where it was actually encouraged for efficiency's sake). As the old saying goes, C programming is done with the blade guards off; you're responsible for taking care of a lot of things which most other languages do for you. C was designed with systems programming in mind, in which there are times when you need to bypass the safety features in order to get something done.

    Most ordinary programming these days (and even in the Bad Old Days when C was designed) doesn't need that sort of power, but for better or worse, a lot of everyday programming is still done in C, and even if you aren't a C programmer, knowing at least a little about C will make you a better programmer in other languages (something that is true of a handful of other languages, along with the likes of assembly, Smalltalk, Forth, the Lisp family, and the pure-FP languages). I don't like the fact that C (and it's descendants C++ and Java) is often used for teaching introductory programming, but it's a fact of life right now.

    But I digress; since you're messing about with DLL loading and unloading, I would expect (or at least hope) that you have good reasons to be working in C.

    Comments on this post

    • sizablegrin agrees
    Last edited by Schol-R-LEA; May 19th, 2009 at 10:28 AM.
    Rev First Speaker Schol-R-LEA;2 JAM LCF ELF KoR KCO BiWM TGIF
    #define KINSEY (rand() % 7) λ Scheme is the Red Pill
    Scheme in Short • Understanding the C/C++ Preprocessor
    Taming Python • A Highly Opinionated Review of Programming Languages for the Novice, v1.1

    FOR SALE: One ShapeSystem 2300 CMD, extensively modified for human use. Includes s/w for anthro, transgender, sex-appeal enhance, & Gillian Anderson and Jason D. Poit clone forms. Some wear. $4500 obo. tverres@et.ins.gov
  26. #14
  27. Devshed God 1st Plane (5500 - 5999 posts)

    Join Date
    Jun 2005
    Posts
    5,964
    Rep Power
    4852
    You're either being lazy or you're befuddled beyond belief. Whichever it is, you need to work on it. You can find solutions here, but we're not going to make your paycheck for you in your actual job.

    Go sit behind the barn, watch the cotton grow, and think about some things.
    Write no code whose complexity leaves you wondering what the hell you did.
    Politically Incorrect DaWei on Pointers Grumpy on Exceptions

IMN logo majestic logo threadwatch logo seochat tools logo