Like Tree1Likes

Help guys with circluarly linked list

This is a discussion on Help guys with circluarly linked list within the C Programming forums, part of the General Programming Boards category; I want to make a circularly linked list taking data from a text file. It keeps saying that my list ...

  1. #1
    Registered User
    Join Date
    Dec 2011
    Posts
    25

    Help guys with circluarly linked list

    I want to make a circularly linked list taking data from a text file.
    It keeps saying that my list is Empty when i try to print it. Is it really empty or is something wrong with the printlist function?


    So far I have written this:

    Code:
    Lics()
    {
        struct p_item *mylist = NULL;
        char letter;
        char word[15];
        int i, j, length;
        FILE *phonetics;
        char c[40];
        
        i=0;
        phonetics=fopen("ICAO.phon", "r");
        if (phonetics == NULL){ 
            printf ("Error opening file\n");
            return;
        }
        else {
            while(!feof(phonetics)) { 
                fscanf(phonetics, "%c", &letter);
                
                fscanf(phonetics, "%s", word);
                
                addnode(mylist, letter, word);
            }
        }
        printf("\n\n");
        printlist(mylist);
        
        return;
    } 
     
    addnode(struct p_item *list, char *letter, char *word[15])
    {
        struct p_item *newnode, *first=list;
        
        if (list==NULL){
            newnode=(struct p_item *)malloc(sizeof(struct p_item));
            newnode->p_letter=letter;
            strcpy(newnode->p_word, word);
            newnode->p_next=newnode;
            return newnode;
        }
        
        while (list->p_next != first){
            list = list->p_next;
            }
            
        newnode = (struct p_item *)malloc(sizeof(struct p_item));
        list->p_next = newnode;
        newnode->p_letter=letter;
        strcpy(newnode->p_word, word);
        newnode->p_next = first;
     
        return first;
        
    }
    
    printlist(struct p_item *list)
    {
        struct p_item *first = list;
        
        if (list == NULL){
            printf("The circularly linked list is empty!\n");
            return;
        }
        do{
            printf("%c %s\n", list->p_letter, list->p_word);
            list = list->p_next;
            } while(list != first);
        
    }

  2. #2
    Registered User
    Join Date
    Nov 2011
    Location
    Saratoga, California, USA
    Posts
    334
    In Lics()? you set mylist = NULL, and in your call to addnode() and in addnode() itself, you never change mylist to point to a struct, so statements like
    Code:
    while (list->p_next != first){        list = list->p_next;
            }
    and
    Code:
    list->p_next = newnode;
    make no sense.

    You are returning values but not catching them anywhere, and your function addnode() doesn't have a return type. So locals newnode and first are lost on exit of addnode() and each time you test your program you are malloc'ing memory which you never free.

  3. #3
    Registered User
    Join Date
    Dec 2011
    Posts
    25
    Thx for your reply but I have to admit that although i see your point, I can't exactly make it happen. Please consider that I have been essentially working on C for about one month (and not that often in that month) so my experience is rather inadequate.

    Could you please point to the proper line of the code and suggest what I should do? This is just the first of my problems but if I can't solve this, I won't be able to go further on.

  4. #4
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,299
    Your first step would be to give your functions return types. Don't just let them default to int. Make sure your compiler warning levels are turned up and pay attention to what they tell you.

    While you're waiting for any other reply, have a read of the following FAQ entry with regards to a different bug that you didn't know you had yet: FAQ > Why it's bad to use feof() to control a loop - Cprogramming.com
    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
    Dec 2011
    Posts
    25
    I must be so inexperienced or so "bad" at C that I still don't understand what I must do in order to get the list to fill itself with what I want. Please show me the way!!

    Btw, I totally get the one about feof (at least I understood that one)

  6. #6
    Registered User
    Join Date
    May 2009
    Posts
    2,543
    Quote Originally Posted by iMalc View Post
    Your first step would be to give your functions return types. Don't just let them default to int.

    Calling a Function and Writing Your Own Functions

  7. #7
    Registered User
    Join Date
    Dec 2011
    Posts
    25
    Quote Originally Posted by Tclausex View Post
    In Lics()? you set mylist = NULL, and in your call to addnode() and in addnode() itself, you never change mylist to point to a struct, so statements like
    Code:
    while (list->p_next != first){        list = list->p_next;
            }
    and
    Code:
    list->p_next = newnode;
    make no sense.
    What should I do about these things mentioned above?

  8. #8
    Registered User
    Join Date
    May 2009
    Posts
    2,543
    Quote Originally Posted by lios1984 View Post
    What should I do about these things mentioned above?
    Fix addnode function so the list head (mylist) is updated.

    Tim S.

  9. #9
    Registered User
    Join Date
    Dec 2011
    Posts
    25
    I have isolated the problem to the part )if (list==NULL). I realize that I can't get to fill the first node of the list. But why? Please consider that you are talking to a C illiterate.

    What should I return;
    Code:
    addnode(struct p_item *list, char *letter, char *word)
    {
        struct p_item *newnode, *first=list;
        
        if (list==NULL){
            printf("Test1");
            list=(struct p_item *)malloc(sizeof(struct p_item));
            list->p_letter=letter;
            strcpy(list->p_word, word);
            list->p_next=first;
            if (list==NULL){
                printf("Error in memory allocation.");
            }
            return list;
        }
        
        while (list->p_next != first){
            list = list->p_next;
            }   
        printf("Test2");
        newnode = (struct p_item *)malloc(sizeof(struct p_item));
        list->p_next = newnode;
        newnode->p_letter=letter;
        strcpy(newnode->p_word, word);
        newnode->p_next = first;
        if (newnode==NULL){
            printf("Error in allocating memory");
        }
        
        return list;
    
        
    }

  10. #10
    Registered User
    Join Date
    Nov 2011
    Location
    Saratoga, California, USA
    Posts
    334
    1. Change addnode so that its return value is 'struct p_item *'

    2. Check for malloc failure BEFORE you attempt to start assigning things.

    3. Don't use implicit cast on malloc.

    4. Realize that your second half of addnode returns the address of the second to last struct in the list.

    5. Your parameter for addnode() should be char letter, not char *letter.

    And lastly, turn you compiler warnings on. Lastly should really be, why are you playing with structs, pointers and linked lists, 1 month out of the gate?
    stahta01 likes this.

  11. #11
    Registered User
    Join Date
    Dec 2011
    Posts
    25
    Guys, don't even ask!! I am trying to complete a course. So far I have managed to get by regarding C. But now with malloc and linked lists, I am way over my head. What I would really appreciate is a step-by-step through my code so that I can finally realize what I am doing so wrong. Ia give you the whole code in case something's wrong with declaring or something else.

    Code:
    #include <stdio.h>
    #include <string.h>
    #include <stdlib.h>
    
    struct p_item
    {
        char p_letter;
        char p_word[15];
        struct p_item *p_next;
    };
    
    typedef struct p_item p_item;
       
    
    int main(int argc, char *argv[])
    {
        char c;
        int i, j;
        
        Lics();
        printf("\n");
        
        system("pause");
        return 1;
    }
    
    Lics()
    {
        struct p_item *mylist = NULL;
        char letter;
        char word[15];
        int i, j, length;
        FILE *phonetics;
        char c[40];
         
        i=0;
        phonetics=fopen("ICAO.phon", "r");
        if (phonetics == NULL){
            printf ("Error opening file\n");
            return;
        }
        else {
            while(!feof(phonetics)) {
                fflush(stdin);
                fscanf(phonetics, "%c", &letter);
                 
                fscanf(phonetics, "%s", word);
                 
                struct p_item *addnode(mylist, letter, word);
            }
        }
        printf("\n\n");
        printlist(mylist);
         
        return;
    }
      
    struct p_item *addnode(struct p_item *list, char *letter, char *word[15])
    {
        struct p_item *newnode, *first=list;
         
        if (list==NULL){
            list=(struct p_item *)malloc(sizeof(struct p_item));
            newnode->p_letter=letter;
            strcpy(newnode->p_word, word);
            newnode->p_next=newnode;
            return newnode;
        }
         
        while (list->p_next != first){
            list = list->p_next;
            }
             
        newnode = (struct p_item *)malloc(sizeof(struct p_item));
        list->p_next = newnode;
        newnode->p_letter=letter;
        strcpy(newnode->p_word, word);
        newnode->p_next = first;
      
        return first;
         
    }
     
    printlist(struct p_item *list)
    {
        struct p_item *first = list;
         
        if (list == NULL){
            printf("The circularly linked list is empty!\n");
            return;
        }
        do{
            printf("%c %s\n", list->p_letter, list->p_word);
            list = list->p_next;
            } while(list != first);
         
    }
    Don't pay attention to the main() part as well as to the !feof part, for now. I mostly want to get over with the list and then the former.

  12. #12
    Registered User
    Join Date
    Dec 2011
    Posts
    25
    Quote Originally Posted by Tclausex View Post
    1. Change addnode so that its return value is 'struct p_item *'

    2. Check for malloc failure BEFORE you attempt to start assigning things.

    3. Don't use implicit cast on malloc.

    4. Realize that your second half of addnode returns the address of the second to last struct in the list.

    5. Your parameter for addnode() should be char letter, not char *letter.

    And lastly, turn you compiler warnings on. Lastly should really be, why are you playing with structs, pointers and linked lists, 1 month out of the gate?
    1. Did it.
    2. I am looking it up right now.
    3. What do you mean?
    4. Theoretically I get it but practically...
    5. When I use char *letter it runs but leads to the error message (The circularly linked list is empty!) and when I use char letter, it doesn't run.

  13. #13
    Registered User
    Join Date
    May 2009
    Posts
    2,543
    Quote Originally Posted by lios1984 View Post
    3. What do you mean?
    FAQ > Casting malloc - Cprogramming.com

    You need to use Function Prototypes and READ YOUR COMPILER warnings!

    http://www.cprogramming.com/tutorial/c/lesson4.html

    The warning I got trying to compile you code
    Code:
    Compiling: main.c
    H:\SourceCode\Projects\testll\main.c: In function 'main':
    H:\SourceCode\Projects\testll\main.c:20:5: warning: implicit declaration of function 'Lics' [-Wimplicit-function-declaration]
    H:\SourceCode\Projects\testll\main.c:18:12: warning: unused variable 'j' [-Wunused-variable]
    H:\SourceCode\Projects\testll\main.c:18:9: warning: unused variable 'i' [-Wunused-variable]
    H:\SourceCode\Projects\testll\main.c:17:10: warning: unused variable 'c' [-Wunused-variable]
    H:\SourceCode\Projects\testll\main.c: At top level:
    H:\SourceCode\Projects\testll\main.c:27:1: warning: return type defaults to 'int' [-Wreturn-type]
    H:\SourceCode\Projects\testll\main.c: In function 'Lics':
    H:\SourceCode\Projects\testll\main.c:40:9: warning: 'return' with no value, in function returning non-void [-Wreturn-type]
    H:\SourceCode\Projects\testll\main.c:49:20: warning: parameter names (without types) in function declaration [enabled by default]
    H:\SourceCode\Projects\testll\main.c:53:5: warning: implicit declaration of function 'printlist' [-Wimplicit-function-declaration]
    H:\SourceCode\Projects\testll\main.c:55:5: warning: 'return' with no value, in function returning non-void [-Wreturn-type]
    H:\SourceCode\Projects\testll\main.c:34:10: warning: unused variable 'c' [-Wunused-variable]
    H:\SourceCode\Projects\testll\main.c:32:15: warning: unused variable 'length' [-Wunused-variable]
    H:\SourceCode\Projects\testll\main.c:32:12: warning: unused variable 'j' [-Wunused-variable]
    H:\SourceCode\Projects\testll\main.c:32:9: warning: variable 'i' set but not used [-Wunused-but-set-variable]
    H:\SourceCode\Projects\testll\main.c: In function 'addnode':
    H:\SourceCode\Projects\testll\main.c:64:26: warning: assignment makes integer from pointer without a cast [enabled by default]
    H:\SourceCode\Projects\testll\main.c:65:9: warning: passing argument 2 of 'strcpy' from incompatible pointer type [enabled by default]
    c:\greenapps\mingw_gcc4_6x_sjlj_tdm\bin\../lib/gcc/mingw32/4.6.1/../../../../include/string.h:45:39: note: expected 'const char *' but argument is of type 'char **'
    H:\SourceCode\Projects\testll\main.c:76:22: warning: assignment makes integer from pointer without a cast [enabled by default]
    H:\SourceCode\Projects\testll\main.c:77:5: warning: passing argument 2 of 'strcpy' from incompatible pointer type [enabled by default]
    c:\greenapps\mingw_gcc4_6x_sjlj_tdm\bin\../lib/gcc/mingw32/4.6.1/../../../../include/string.h:45:39: note: expected 'const char *' but argument is of type 'char **'
    H:\SourceCode\Projects\testll\main.c: At top level:
    H:\SourceCode\Projects\testll\main.c:84:1: warning: return type defaults to 'int' [-Wreturn-type]
    H:\SourceCode\Projects\testll\main.c: In function 'printlist':
    H:\SourceCode\Projects\testll\main.c:90:9: warning: 'return' with no value, in function returning non-void [-Wreturn-type]
    H:\SourceCode\Projects\testll\main.c: In function 'addnode':
    H:\SourceCode\Projects\testll\main.c:64:26: warning: 'newnode' may be used uninitialized in this function [-Wuninitialized]
    Tim S.
    Last edited by stahta01; 12-13-2011 at 11:45 AM.

  14. #14
    Registered User
    Join Date
    Dec 2011
    Posts
    25
    !!!!
    I got only 2 warnings whenever I compiled. I use wx-Dev-C++. Maybe that's because I don't get all these warnings? Or are they turned off? I can't find any settings.

    How do I turn them on?

    I'll stay up tonight and try to solve this. So I'll be online..

  15. #15
    Registered User
    Join Date
    Nov 2011
    Location
    Saratoga, California, USA
    Posts
    334
    Quote Originally Posted by lios1984 View Post
    .
    4. Theoretically I get it but practically...
    Well, you wrote the code to move 'list' up to the last struct in the linked list, and then malloc a new struct, and point list->p_next to that new struct. So now list points to that second-to-last struct and that memory location is what your are returning to main to be assigned to 'mylist'. (At least I assume you're assigning it to mylist).

    I guess I'm asking if that's really what you want. Think about it for a few minutes. You never really have a "first" item in the linked list, it's constantly changing. And every addnode() call treats that position as "first" and then runs around the ring until it is behind "first". As your list grows, you are literally running around in circles for no good reason. You need to decide if this is really the behavior you want, or do you want the first struct added to really be, and remain the "first" node.

    Quote Originally Posted by lios1984 View Post
    .
    5. When I use char *letter it runs but leads to the error message (The circularly linked list is empty!) and when I use char letter, it doesn't run.
    Trust me, you can use char *letter, but it's not called for here. You use a pointer as parameter in a function when you want to change the variable in main() from within said function. You aren't changing letter, so a pointer is uncalled for. And IF you wanted to use a pointer, the value you pass to it with the function call must be an address, i.e. use the & address operator.
    Using the correct formal parameter of 'char letter' is certainly NOT what is preventing your program from working.

    Finally, I see you removed the malloc error-checking altogether . Put it back or puppies will die. The error checking should come right after the malloc call, otherwise you attempt to assign values to a struct that doesn't exist. And if malloc returns NULL, frankly, you should end your program with an error statement and an exit(1) call.

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

Similar Threads

  1. Replies: 4
    Last Post: 05-01-2010, 10:19 PM
  2. single linked list to double linked list (help)
    By Countfog in forum C Programming
    Replies: 8
    Last Post: 04-29-2008, 08:04 PM
  3. Replies: 11
    Last Post: 01-02-2006, 03:46 PM
  4. singly linked list to doubly linked list
    By t48j in forum C Programming
    Replies: 3
    Last Post: 03-23-2005, 05:37 PM
  5. Replies: 6
    Last Post: 03-02-2005, 01:45 AM

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