0

Having a lot of trouble with the unload() function for trie. I have been trying to fix for it for sometime now but haven't been able to. The code when implemented shows the misspelled words, but before the information containing time and all is shown, it shows segmentation fault. Even while running valgrind it shows memory leaked as zero but at the end shows segmentation fault.

Any and all help is appreciated!

// Implements a dictionary's functionality

#include<stdio.h>
#include<stdlib.h>
#include<stdbool.h>
#include<string.h>
#include<ctype.h>
#include "dictionary.h"

int ctr=0,e=0;
typedef struct node
{
    bool is_word;
    struct node *children[27];
}
node;
node *root;
node *l;

bool del(node *n);
// Returns true if word is in dictionary else false
bool check(const char *word)
{
  int j;
  node *p=root;
  int n,d;
  n=strlen(word);
  char w[50];
  for(j=0;j<n;j++)
      w[j]=tolower(word[j]);
  w[j]='\0';
  for(int i=0;i<n;i++)
  {
    d=(int)w[i]%97;
    if(d==39)
        d=26;
    if(p->children[d]==NULL)
        return false;
    else
        p=p->children[d];
  }
  if(p->is_word==true)
      return true;
  else
      return false;
}

// Loads dictionary into memory, returning true if successful else false
bool load(const char *dictionary)
{
      root=malloc(sizeof(node));
      for(int m=0;m<27;m++)
   {
       root->children[m]=NULL;
   }
   node *p=root;
   int d;
   FILE *file = fopen(dictionary,"r");
   if (file == NULL)
   {
         printf("Could not open %s.\n", dictionary);
         unload();
         return false;
   }
   for (int c = fgetc(file); c != EOF; c = fgetc(file))
   {
        if(c!='\n')
        {
            d=(int)c%97;
            if(d==39)
                d=26;
            if(p->children[d]==NULL)
            {
                node *t=malloc(sizeof(node));
                for(int m=0;m<27;m++)
                {
                    t->children[m]=NULL;
                }
                p->children[d]=t;
                p=p->children[d];
            }
            else
            {
                 p=p->children[d];
            }
        }
        else
        {
            p->is_word=true;
            p=root;
            ctr++;
        }
    }
    return true;
}

// Returns number of words in dictionary if loaded else 0 if not yet loaded
unsigned int size(void)
{
     return ctr;
}

// Unloads dictionary from memory, returning true if successful else false
bool unload(void)
{
     if(e==0)
        l=root;
     e++;
     for(int i=0;i<27;i++)
     {
           if(l->children[i]!=NULL)
           {
                node *o=l;
                l=l->children[i];
                if(del(o->children[i]))
                o->children[i]=NULL;
                unload();
           }
     }
     node *t=l;
     if(l==root)
         root=NULL;
     l=NULL;
     free(t);
     if(root!=NULL)
     {
          l=root;
          unload();
     }
     return true;
}



bool del(node *n)
{
      for(int i=0;i<27;i++)
      {
           if(n->children[i]!=NULL)
               return false;
      }
      return true;
}
1

In the unload() code, it has the following line that is causing the seg fault.

       if(l->children[i]!=NULL)

The reason for the seg fault is this. At some point, l is null, so l->children[i] doesn't exist. That causes a seg fault. I'll leave it to you to sort out the logic.

I didn't delve into the logic for a couple of reasons. First, there are simply too many single letter variables to easily follow the logic. The use of single letter variables for anything more than a simple for loop counter is a bad practice. It makes code difficult to follow and debug. Try finding all the uses of the variable e in your code vs. the use of the letter e anywhere in the code! This is the single largest reason I chose not to analyze the code to any degree. Best practice is to use descriptive variable names for each var. This will also help others (or yourself six months later) to debug and update code over time.

More importantly, code complexity generally leads to problems down the road - more opportunities for bugs to appear in code and more difficulty analyzing and modifying or updating code at a later date. It appears that you have used several vars to track the number of nodes throughout the code or for other purposes, I'm not sure. When building trees/tries or linked lists, its better to walk the tree and delete as you go than depending on a counter. With a walk, you're tracing the tree. With a counter, you may get off track and lose part of the tree, along with adding extraneous code. (I'll still grant that tracking the number of freed nodes should match the number of created nodes when finished makes a reasonable check.)

Finally, if a value is used for the same purpose throughout the code, it should be declared in one place (at the top of the .c file or in a .h file, as appropriate) and the declared name should be used in the code. What if that value has to be changed at some point? How would a programmer be sure that all the instances should be changed? This is the "magic number" discussion in the lectures. A number that is used more than once for the same purpose should be declared. For example, #delcare MAXWORDLENGTH 50 could be declared and then the constant name MAXWORDLENGTH can be used throughout the code. The compiler will insert 50 wherever it is used, but if the code ever needs to be changed, it only needs to be changed in the declaration.

This should get you going again. If there are other issues, please open a new question.

If this answers your question, please click on the check mark to accept. Let's keep up on forum maintenance. ;-)

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .