Thread: Linked Lists. Argh!!!

I am really having issues with getting linked list to work. My assignment requires us to open a txt file and read the words in to a linked list of structs. I thought i had my list sorted but i can not seem to read back from it. I have read the FAQ section here and whilst concise i was not able to find a solution. If anybody would have a second i would really appreciate if you could have a quick look at my code.

Code:

/* Structure definitions. */
typedef struct wordStruct* WordTypePtr;

typedef struct wordStruct
{
   char* word;
   unsigned count;
   WordTypePtr nextWord;
} WordType;

typedef struct indexStruct
{
   WordTypePtr headWord;
   WordTypePtr longestWord;
   WordTypePtr mostCommonWord;
   unsigned totalWords;
   unsigned uniqueWords;
} IndexType;

Code:

/* part of main */
int main(int argc, char* argv[])
{
   IndexType index;
   WordType  * head;
   
   index.headWord = head;
   head=NULL;
   
   char myString[MENU_SIZE+2];
   int finished = FALSE;
   int i;
   int x;
   int menuActive = TRUE;
   char fname[99];
   index.totalWords=0;
   if (argc==1){
                      printf("\nError: No file specified\n");
                      system("PAUSE");	
                      return EXIT_SUCCESS;
                      }   
   strcpy(fname,argv[1]);
x=loadData(&index, fname);

Code:

/****************************************************************************
* Loads all data into the index.
****************************************************************************/
int loadData(IndexType* index, char* trecFile)
{
    
    
    
    
     WordType * curr, * head;
    /*-------------------------*/
    
    head=index->headWord;
char wordvar[99];
int c,j; 
int d=0;

j=0;
   FILE *fp;
    
    printf("%s\n",trecFile);
        /*Name of file */
       /* File pointer- FILE defined in <stdio.h> */
   fp = fopen(trecFile, "r"); /* File open function for reading */ 
 
/* Checking the return value of fopen() */
   if ( fp == NULL) /* NULL defined in <stdio.h> */ 
   {
      fprintf(stderr, "\nUnable to open file %s\n", trecFile); /* Redirect output to stderr */
      system("PAUSE");	
      return EXIT_SUCCESS;
   }
   else
   {
      /* Safe to read from file */
      printf("\nFile %s open\n",trecFile);
      while(( c = getc(fp)) != EOF)
      {
         if(c=='<'){
                    d=1;
                    }
         if (d==1){
                              
         }else{
               if(tolower(c)>96 && tolower(c) <123){
                                printf("%c",tolower(c));
                                wordvar[j]=tolower(c);
                                j++;
                                }
         
         if (c==' '){
                  index->totalWords++;
                  curr = (WordType *)malloc(sizeof(WordType));
                  curr->count++;
                  curr->word=wordvar;
                  printf("\n%s\n",wordvar);
                  printf("%s\n",curr->word);
                  curr->nextWord  = head;
                  /*printf(" pointer-> %c/n",head);*/
                  head = curr;
                  printf("\n");/*
                  j=0;*/
                     for(j=0;j<=99;j++) {
                     wordvar[j]=NULL;
                                          }
                                          j=0;
                  }
                  
         }
         if(c=='>'){
                    d=0;
                    }
      }

   }
   
      fclose(fp);/* closes stream fp */
      printf("Total Words: %d\n\n",index->totalWords);
      
      curr = head;
      printf("%s\n",curr->word);
      curr = curr->nextWord ;
      printf("%s\n",curr->word);
      for(j=0;j<=9;j++) {
                 printf("%s\n",curr->word);
      curr = curr->nextWord ;  
                                          }
                                          
         /*while(curr) {
      printf("%s\n", curr->word);
      curr = curr->nextWord ;
   }*/
/*       
curr = head;

   while(curr) {
      printf("%s\n", curr->word);
      curr = curr->nextWord ;
   }*/
   
    return SUCCESS;
}

Have you ever successfully made a linked list? For example, you should first attempt to make a linked list with a small set of numbers -- hard coded or entered by you. Basic linked list functions are simple to make into any type AFTER you figure it out. Pause what you are doing and dumb down the problem. Make a SIMPLE linked list, and after you get it working, merge the two.

Thanks Kennedy,

my original base code works fine with integers.

Code:

#include<stdlib.h>
#include<stdio.h>
void printlist1();
struct list_el {
   int val;
   struct list_el * next;
};

typedef struct list_el item;

void main() {
   item * curr, * head;
   int i;
   printlist1();
   head = NULL;

   for(i=1;i<=50;i++) {
      curr = (item *)malloc(sizeof(item));
      curr->val = i+9;
      curr->next  = head;
      head = curr;
   }

   curr = head;


   printlist1(curr);
   system("PAUSE");    
}

void printlist1(item * curr)
{
     printf("Print\n");
        while(curr) {
      printf("%d\n", curr->val);
      curr = curr->next ;
   }
     }

Ollie,

I just wrote a very thoughtful reply for the thread Ollie, and the browser lost it after complaining about my log in. This is my second attempt to reply.

I looked over your code, and I've got some concerns about the loop in the loadData function. After I fixed the indention, so I could read it, and numbered the lines with cat, I got:

Code:

     1	      while(( c = getc(fp)) != EOF)
     2	      {
     3	         if(c=='<')
     4	         {
     5	            d=1;
     6	         }
     7	         
     8	         if (d==1)
     9	         {
    10	                              
    11	         }
    12	         else
    13	         {
    14	            if(tolower(c)>96 && tolower(c) <123)
    15	            {
    16	               printf("%c",tolower(c));
    17	               wordvar[j]=tolower(c);
    18	               j++;
    19	            }
    20	         
    21	            if (c==' ')
    22	            {
    23	               index->totalWords++;
    24	               curr = (WordType *)malloc(sizeof(WordType));
    25	               curr->count++;
    26	               curr->word=wordvar;
    27	               printf("\n%s\n",wordvar);
    28	               printf("%s\n",curr->word);
    29	               curr->nextWord  = head;
    30	               /*printf(" pointer-> %c/n",head);*/
    31	               head = curr;
    32	               printf("\n");/*
    33	               j=0;*/
    34	               
    35	               for(j=0;j<=99;j++)
    36	               {
    37	                  wordvar[j]=NULL;
    38	               }
    39	               j=0;
    40	            }
    41	                  
    42	         }
    43	
    44	         if(c=='>')
    45	         {
    46	            d=0;
    47	         }
    48	      }

Line 8/12: Maybe replace with if( 1 != d )?

Line 14/21: Maybe replace second if with else if because they are exclusive?

Line 26: Memory leak there. I'm confident that you intended to make a deep copy of the word that was just read. For example:

Code:

curr->word = (char*) malloc ( 99*sizeof( char ) );
strncpy( curr->word, wordvar, 98 );

That way the node gets it's own copy of the word from input rather than referring to the local variable wordvar.

Line 35: The end points on the loop...wordvar is 99 characters long, they are indexed from 0-98. The loop actually runs for one too many, i.e. 0-99.

Line 37: It seemed a little awkward to use the NULL constant to clear the buffer. It's the same value as '\0' or (char)0, but it's connotation is generally applied to pointer values rather than a character...just a style thing I guess.

Speaking of style, the Wikipedia has a great article about it: Programming style - Wikipedia, the free encyclopedia. I personally attempt to apply the Linux kernel coding style: LXR / The Linux Cross Reference.

Over all, very nice Ollie. I realize that indention gets skew when one is hammering at the debugger. I suggest using a flow chart to document the flow control of the code section to verify that it is doing what it should be. It usually only takes a minute, and it can be much clearer than reading the code to myself.

I hope that my review helps. I hope to check back in with you soon.

Best Regards,

New Ink -- Henry

Thanks to all, especially new ink, i think i hust had a bit of a lightbulb moment. I will take a look at my code later today. I just wanted to take this moment to thank everybody for taking the time so far to have a look at my issue.

Cheers
Ollie