Help With Segmentation Error

I'm currently building a C Program however I'm running into a segmentation error whenever I delete an entry from my program and attempt to re-execute.

An example is when I add two names and delete one and then attempt to order them alphabetically again. My program terminates and after debugging it seems to be due to a segmentation error

Any insight into the issue would be greatly appreciated. I can't seem to see what the issue is.

I apologize for any indentation errors as in my compiler they are not present.

Code:

---

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>


//Function prototype declarations


void addentry();
void delentry();
void showphonebook();
void alphabeticalsort();
void find();
void rancontact();
void delall();




typedef struct phonebook //First data structure is built for storing names and phonenumbers; not sure if this method is most efficient but it does seem practical


{


        char *FirstName;
        char *LastName;
        char *number;


} book;


typedef struct delbook //Second data structure for deleting user data


{


        char *FirstName;
        char *LastName;       


} del;


//Pointer and global variable initializations
book *bk;
del *dl;
int count = 0;
int delcount = 0;


int main(void)


{
     
int menu;


      do //do while loop for my menu, always a good preference for one initialized run; helpful when troubleshooting
     
      { 


          printf("\nPhone Book Application\n");
          printf("1) Add Friend\n");
          printf("2) Delete Friend\n");
          printf("3) Display Phone Book\n");
          printf("4) Alphabetically Sort\n");
          printf("5) Find Name\n");
          printf("6) Call Random Contact\n");
          printf("7) Delete Entire Phonebook\n");
          printf("8) Exit\n");
          printf("\nWhat do you want to do?: ");
          scanf("%d", &menu);


       


          switch (menu)
         
                        {
                        case 1:
                            addentry();
                            count++;
                            break;


                        case 2:
                            delentry();
                            break;


                        case 3:
                            showphonebook();
                            break;
                           
                        case 4:
                            alphabeticalsort();
                            break;
                           
                        case 5:
                            find();
                            break;
                           
                        case 6:   
                            rancontact();
                            break;
                           
                        case 7:   
                            delall();
                            break;
                           
                        case 8:
                            printf("\nThank you for using this program! Good Bye!"); //User friendly message for program exit
                              break;
                         
                        default: //Thought using default case instead of while loop would be a bit neater from logic used in previous programs
                              printf("\nPlease enter a valid menu selection.\n");
                            break;   
                      }


                     


      } while (menu != 8); //End of do while loop based of exiting criteria
                              //Thought I'd try some different recursive logic for reinitializing menu


         


//Returning memory here for efficiency
free(dl);
free(bk);
dl = NULL;
bk = NULL;         


return 0;
}


//Begin Functions


//Function for adding entries into phonebook
void addentry()


{
    char *lname;


    if (count == 0)
        {
          bk = (book *) malloc ((count*100) + 100); //allocating memory with arbitrarily large number
        }


    else
        {
          bk = (book *) realloc (bk, (count*100) + 100); //reallocation of memory with arbitrarily chosen large number
        }


    if (bk == NULL)    //Statement in the event memory is unavailable for new entries
        {
          printf("No memory available for allocation\n");
        }


    else //Here we go for adding an entry!
        {
       
          bk[count].FirstName = (char *)malloc(sizeof(char)*100);
          bk[count].LastName = (char *)malloc(sizeof(char)*100);
          bk[count].number = (char *)malloc(sizeof(char)*100);
          //Adding some spacing here for eye relief
          printf("First Name: ");
          scanf("%s", bk[count].FirstName);
          printf("Last Name: ");
          scanf("%s", bk[count].LastName);
          printf("Phone Number: ");
          scanf("%s", bk[count].number);
          printf("Record added to the Phone Book\n");


        }


   
}


//Function for deleting user entries in phonebook
void delentry()


{
    int i, k;
    char *name;


    if (delcount == 0)
    {
          dl = (del *) malloc ((delcount*50) + 50);
    }


    else
    {
          dl = (del *) realloc (dl, (delcount*100) + 100); //Potential memory segmentation error here with realloc; requires further investigation
    }


    if (dl == NULL)
    {
        printf("Out of memory can not Delete\n");
    }
    else
    {
          dl[delcount].FirstName = (char *) malloc(sizeof(char)*50);
          dl[delcount].LastName = (char *) malloc(sizeof(char)*50);
          printf("First Name: ");
          scanf("%s", dl[delcount].FirstName);
          printf("Last Name: ");
          scanf("%s", dl[delcount].LastName);
    }


    for (i = 0; i < count; i++)
    {
        if (bk[i].FirstName == NULL && bk[i].LastName == NULL) continue;
      if (strcmp(bk[i].FirstName, dl[delcount].FirstName) == 0 && strcmp(bk[i].LastName, dl[delcount].LastName) == 0)
              {
                    printf("Record has been deleted\n");
                  bk[i].FirstName = NULL;
                  bk[i].LastName = NULL;
                    bk[i].number = NULL;
                    k = 1;
                  break;
              }
    }


        if (k != 1)
        {
            printf("\n Entry not found.\n");
        }


        delcount++;




}         


void showphonebook()


{
    int i;


    printf("\nPhone Book Entries:\n");


    for (i = 0; i < count; i++)


        {
            if (bk[i].FirstName != NULL && bk[i].LastName != NULL)
                    {
                        printf("\n%s %s %s\n", bk[i].FirstName, bk[i].LastName, bk[i].number);
                    }


        }
    printf("\n"); //Added for spacing for eye relief between contacts being printed
}


void alphabeticalsort() //Alphabetically sorts contacts by last name just like normal phonebook programs


{
      int i, k;
      char *fname;
     
      char *t1 = (char*)malloc(sizeof(char)*15);
      char *t2 = (char*)malloc(sizeof(char)*15);
      char *t3 = (char*)malloc(sizeof(char)*15);




/*I addmitedly had a lot of trouble on the logic in this part and I'm still not confident this is the most efficient method.
I initally realized I was only comparing and reassigning last name strings as opposed to all user data.
My way around that issue was to declare the 3 temporary strings for storage of temporary data during the sort.*/


//I also seem to run into a segmentation error when I run this section after deleting all names from phonebook and readding


     
      for (i = 0; i < count; i++)
        {
           
            for (k = i + 1; k < count; k++)
                {
                   
                    if (strcmp(bk[i].LastName,bk[k].LastName) > 0) //My if statement to manually sort user data in alphabetical order by last name
                   
                        {
                            strcpy(t1, bk[k].FirstName);
                              strcpy(t2, bk[k].LastName);
                              strcpy(t3, bk[k].number);
         
                              strcpy(bk[k].FirstName, bk[i].FirstName);
                              strcpy(bk[k].LastName, bk[i].LastName);
                              strcpy(bk[k].number,bk[i].number);
         
                              strcpy(bk[i].FirstName, t1);
                              strcpy(bk[i].LastName, t2);
                              strcpy(bk[i].number,t3);
                             
                        }
                   
                }
               
        }   
       
        for (i = 0; i < count; i++)
            {
                printf("\n%s %s: %s\n", bk[i].FirstName, bk[i].LastName, bk[i].number);
            }
           
           
       
       
}               


 void find() //function to search all users in phonebook
 
 {
   
    int i;
    int k = 0;
    char first[50];
    char last[50];
   
    printf("Enter the person's first name:\n");
    scanf("%s", first);
    printf("Enter the person's last name:\n");
    scanf("%s", last);
   
    for (i = 0; i < count; i++)
        {
           
          if (strcmp(bk[i].FirstName, first) == 0 && strcmp(bk[i].LastName, last) == 0) //If statement for finding the person of interest
               
            {
               
                printf("\n%s %s's number is: %s\n", bk[i].FirstName, bk[i].LastName, bk[i].number);
               
                k = 1;
               
                break;
               
            }
       
        }
   
    if (k != 1) //User friendly syntax here to let them know their person of interest could not be found
        {
       
            printf("This person's record does not exist\n");
       
        }
   
 }
 
  void rancontact() //Function to print random contact information
 
  {


      srand(time(NULL));
      int num;
     
      num = rand()%count;
     
      printf("%s %s %s \n", bk[num].FirstName, bk[num].LastName, bk[num].number);
     
     
  }
 
  void delall()
 
  {
     
     
      int i, menu;
     
      printf("Are you sure you want to continue? Your phonebook will be completely deleted if you continue.\n"); //Freindly user syntax here to make sure they don't accidentally delete everything
      printf("To confirm you want to continue please press 1 otherwise enter any other number\n");
      printf("What is your selection:");
      scanf("%d", &menu);
     
      if (menu == 1)
        {
             
          for (i = 0; i < count; i++)
     
              {
             
                  do {  //deleting everything with null assignments
                 
                      bk[i].FirstName = NULL;
                      bk[i].LastName = NULL;
                      bk[i].number = NULL;
                 
                      break;
                 
                    } while (i <= count);
     
              }
         
          printf("\nYour phonebook has been completely deleted\n"); //Friendly user message here
         
          }
         
      printf("\nYou will now return to the main menu.\n"); //Friendly user message
  }

---

Noticed that sometimes names are allocated 50 bytes, other times 100. Could that be your problem? Maybe add "#define MAX_NAME_LEN 100", and use that for consistency.

Code:

---

  for (i = 0; i < count; i++)
  {
      if (bk[i].FirstName == NULL && bk[i].LastName == NULL) continue;
      if (strcmp(bk[i].FirstName, dl[delcount].FirstName) == 0 && strcmp(bk[i].LastName, dl[delcount].LastName) == 0)
      {
        printf("Record has been deleted\n");
        bk[i].FirstName = NULL;
        bk[i].LastName = NULL;
        bk[i].number = NULL;
        k = 1;
        break;
      }
  }

---

You should be 'free()'ing the names before you assign them to NULL.

I also don't know why you keep 'dl' - an array containing all the names that have been deleted. I would expect that that you have the firstName and lastName of the item being deleted on the stack.

Lookup strncpy and use it instead of strcpy.
Decide if you are programming in "C" or C++; then write C or C++.

Casting malloc is wrong in "C"; but, often needed in C++!
Stop adding blank lines for no reason.

EDIT: stop using magic numbers like "15", "50", and "100".
Use define or const instead.

Tim S.

I agree that I was a little sloppy with the number of bytes being allocated. I decided to use the define directive to keep my byte allocations constant across the board. Still experiencing a program crash when attempting to call the alphabetical sorting function after deleting and then re-adding someone's information. It works perfectly fine if I don't delete anyone's name.

I also questioned my efficiency with dl and wondered if I should try and build a different delete function based solely on the bk struct. I appreciate your insight.

Code:

---

srand(time(NULL));

---

That code needs to be called only once!
So, normally please in main function or init function.

Tim S.

Ok so after some investigating I figured out that not decrementing count after deleting left null strings available for sorting. hamster_nz was correct in freeing everything before assigning NULL in both single delete function and delete all function.

However, now after implementing count-- in my single delete function it seems that if I delete a name in the middle of the list it automatically decrements and deletes any string afterwards as well.

Example:
//

Bob
Joe
Jim

Select Joe for deletion

Output:
Bob
//

My brain is a little fried from debugging. Any insight how to preserve the strings that were input after the one designated for deletion?

Here is my code now.

Code:

---


#include <stdio.h>#include <stdlib.h>
#include <string.h>
#include <time.h>


#define Max_Name_Length 50
//Function prototype declarations


void addentry();
void delentry();
void showphonebook();
void alphabeticalsort();
void find();
void rancontact();
void delall();




typedef struct phonebook //First data structure is built for storing names and phonenumbers; not sure if this method is most efficient but it does seem practical


{


        char *FirstName;
        char *LastName;
        char *number;


} book;


typedef struct delbook //Second data structure for deleting user data


{


        char *FirstName;
        char *LastName;       


} del;


//Pointer and global variable initializations
book *bk;
del *dl;
int count = 0;
int delcount = 0;


int main(void)


{
     
int menu;


      do //do while loop for my menu, always a good preference for one initialized run; helpful when troubleshooting
         
          { 


          printf("\nPhone Book Application\n");
          printf("1) Add Friend\n");
          printf("2) Delete Friend\n");
          printf("3) Display Phone Book\n");
          printf("4) Alphabetically Sort\n");
          printf("5) Find Name\n");
          printf("6) Call Random Contact\n");
          printf("7) Delete Entire Phonebook\n");
          printf("8) Exit\n");
          printf("\nWhat do you want to do?: ");
          scanf("%d", &menu);


       


          switch (menu)
                 
                                            {
                        case 1:
                            addentry();
                            count++;
                            break;


                        case 2:
                            delentry();
                            break;


                        case 3:
                            showphonebook();
                            break;
                           
                        case 4:
                                                        alphabeticalsort();
                                                        break;
                                                       
                                                case 5:
                                                        find();
                                                        break;
                                                       
                                                case 6:       
                                                        rancontact();
                                                        break;
                                                       
                                                case 7:       
                                                        delall();
                                                        break;
                                                       
                                            case 8:
                                                    printf("\nThank you for using this program! Good Bye!"); //User friendly message for program exit
                                                          break;
                                                 
                        default: //Thought using default case instead of while loop would be a bit neater from logic used in previous programs
                                                          printf("\nPlease enter a valid menu selection.\n");
                            break;   
                                  }


                     


      } while (menu != 8); //End of do while loop based of exiting criteria
                                                      //Thought I'd try some different recursive logic for reinitializing menu


         


//Returning memory here for efficiency
free(dl);
free(bk);
dl = NULL;
bk = NULL;         


return 0;
}


//Begin Functions


//Function for adding entries into phonebook
void addentry()


{
    char *name;


    if (count == 0)
            {
          bk = (book *)malloc((count*Max_Name_Length) + Max_Name_Length); //allocating memory with arbitrarily large number
            }


    else
            {
          bk = (book *)realloc(bk, (count*Max_Name_Length) + Max_Name_Length); //reallocation of memory with arbitrarily chosen large number
            }


    if (bk == NULL)        //Statement in the event memory is unavailable for new entries
            {
          printf("No memory available for allocation\n");
            }


    else //Here we go for adding an entry!
            {
           
          bk[count].FirstName = (char *)malloc(sizeof(char)*Max_Name_Length);
          bk[count].LastName = (char *)malloc(sizeof(char)*Max_Name_Length);
          bk[count].number = (char *)malloc(sizeof(char)*Max_Name_Length);
                  //Adding some spacing here for eye relief
          printf("First Name: ");
          scanf("%s", bk[count].FirstName);
          printf("Last Name: ");
          scanf("%s", bk[count].LastName);
          printf("Phone Number: ");
          scanf("%s", bk[count].number);
          printf("Record added to the Phone Book\n");


            }


   
}


//Function for deleting user entries in phonebook
void delentry()


{
        int i, k;
        char *name;


        if (delcount == 0)
        {
                  dl = (del *)malloc((delcount*Max_Name_Length) + Max_Name_Length);
        }


        else
        {
                  dl = (del *)realloc(dl, (delcount*Max_Name_Length) + Max_Name_Length);
        }


        if (dl == NULL)
        {
                printf("Out of memory can not Delete\n");
        }
        else
        {
                  dl[delcount].FirstName = (char *)malloc(sizeof(char)*Max_Name_Length);
                  dl[delcount].LastName = (char *)malloc(sizeof(char)*Max_Name_Length);
                  printf("First Name: ");
                  scanf("%s", dl[delcount].FirstName);
                  printf("Last Name: ");
                  scanf("%s", dl[delcount].LastName);
        }


    for (i = 0; i < count; i++)
        {
            if (bk[i].FirstName == NULL && bk[i].LastName == NULL) continue;
      if (strcmp(bk[i].FirstName, dl[delcount].FirstName) == 0 && strcmp(bk[i].LastName, dl[delcount].LastName) == 0)
                          {
                                    printf("Record has been deleted\n");
                                    free(bk[i].FirstName);
                                  free(bk[i].LastName);
                                  free(bk[i].number);
                                  bk[i].FirstName = NULL;
                                  bk[i].LastName = NULL;
                                    bk[i].number = NULL;
                                    k = 1;
                                    count--;
                              break;
                          }
        }


                if (k != 1)
                {
                        printf("\n Entry not found.\n");
                }


                delcount++;




}         


void showphonebook()


{
    int i;


    printf("\nPhone Book Entries:\n");


    for (i = 0; i < count; i++)


            {
                if (bk[i].FirstName != NULL && bk[i].LastName != NULL)
                                {
                        printf("\n%s %s %s\n", bk[i].FirstName, bk[i].LastName, bk[i].number);
                                }


            }
        printf("\n"); //Added for spacing for eye relief between contacts being printed
}


void alphabeticalsort() //Alphabetically sorts contacts by last name just like normal phonebook programs


{
          int i, k;
         
          char *t1 = (char*)malloc(sizeof(char)*Max_Name_Length);
      char *t2 = (char*)malloc(sizeof(char)*Max_Name_Length);
      char *t3 = (char*)malloc(sizeof(char)*Max_Name_Length);




/*I addmitedly had a lot of trouble on the logic in this part and I'm still not confident this is the most efficient method.
I initally realized I was only comparing and reassigning last name strings as opposed to all user data.
My way around that issue was to declare the 3 temporary strings for storage of temporary data during the sort.*/


//I also seem to run into a segmentation error when I run this section after deleting all names from phonebook and re-adding
         
          for (i = 0; i < count; i++)
                {
                       
                        for (k = i + 1; k < count; k++)
                                {
                                       
                                        if (strcmp(bk[i].LastName,bk[k].LastName) > 0) //My if statement to manually sort user data in alphabetical order by last name
                                       
                                                {
                                                        strcpy(t1, bk[k].FirstName);
                                                  strcpy(t2, bk[k].LastName);
                                                  strcpy(t3, bk[k].number);
         
                                                  strcpy(bk[k].FirstName, bk[i].FirstName);
                                                  strcpy(bk[k].LastName, bk[i].LastName);
                                                  strcpy(bk[k].number,bk[i].number);
         
                                                  strcpy(bk[i].FirstName, t1);
                                                  strcpy(bk[i].LastName, t2);
                                                  strcpy(bk[i].number,t3);
                                                 
                                                }
                                       
                                }
                               
                }       
               
                for (i = 0; i < count; i++)
                        {
                                printf("\n%s %s: %s\n", bk[i].FirstName, bk[i].LastName, bk[i].number);
                        }
                       
                       
               
               
}                               


 void find() //function to search all users in phonebook
 
 {
       
        int i;
        int k = 0;
        char first[Max_Name_Length];
        char last[Max_Name_Length];
       
        printf("Enter the person's first name:\n");
        scanf("%s", first);
        printf("Enter the person's last name:\n");
        scanf("%s", last);
       
        for (i = 0; i < count; i++)
                {
                       
                  if (strcmp(bk[i].FirstName, first) == 0 && strcmp(bk[i].LastName, last) == 0) //If statement for finding the person of interest
                               
                        {
                               
                                printf("\n%s %s's number is: %s\n", bk[i].FirstName, bk[i].LastName, bk[i].number);
                               
                                k = 1;
                               
                                break;
                               
                        }
               
                }
       
        if (k != 1) //User friendly syntax here to let them know their person of interest could not be found
                {
               
                        printf("This person's record does not exist\n");
               
                }
       
 }
 
  void rancontact() //Function to print random contact information
 
  {


          srand(time(NULL));
          int num;
         
          num = rand()%count;
         
          printf("%s %s %s \n", bk[num].FirstName, bk[num].LastName, bk[num].number);
         
         
  }
 
  void delall()
 
  {
         
         
          int i, menu;
         
          printf("Are you sure you want to continue? Your phonebook will be completely deleted if you continue.\n"); //Freindly user syntax here to make sure they don't accidentally delete everything
          printf("To confirm you want to continue please press 1 otherwise enter any other number\n");
          printf("What is your selection:");
          scanf("%d", &menu);
         
          if (menu == 1)
                {
                         
                  for (i = 0; i < count; i++)
         
                          {
                         
                                  do {  //deleting everything with null assignments
                                          free(bk[i].FirstName);
                                          free(bk[i].LastName);
                                          free(bk[i].number);
                                          bk[i].FirstName = NULL;
                                          bk[i].LastName = NULL;
                                          bk[i].number = NULL;
                                 
                                          break;
                                 
                                    } while (i <= count);
         
                          }
                 
                  printf("\nYour phonebook has been completely deleted\n"); //Friendly user message here
                 
                  }
                 
          printf("\nYou will now return to the main menu.\n"); //Friendly user message
  }

---

Your issue is with this line in the sort:

Code:

---

if (strcmp(bk[i].LastName,bk[k].LastName) > 0) //My if statement to manually sort user data in alphabetical order by last name

---

If either 'LastName' is NULL (after the deletion sometimes one will be) this will be calling strcmp() with a NULL pointer.

The easy fix would be to check for NULL pointers before doing the comparison.

The better fix would be when you delete an item, if it isn't the last item (i.e. the index is count-1) then move the last item into this place, and reduce count. However this will change the ordering.

The best fix would be to move all the items after the one that has been deleted one closer to the start of the array, then reduce count by one.

The last two solutions avoid having empty items in the array, so you shouldn't need to guard against NULL pointers.

Other ideas: You want to move "count++;" from the menu processing code into the addentry() function. It should only be incremented if a new item is added.

Oh, also in the sort, you don't need to move the strings around, just the pointers. That removes the need to malloc more memory:

Code:

---

  for (i = 0; i < count; i++) {
      for (k = i + 1; k < count; k++) {
        if(bk[i].LastName == NULL ||bk[k].LastName == NULL) // Avoid segfaults with deleted items
            continue;


        //My if statement to manually sort user data in alphabetical order by last name
        if (strcmp(bk[i].LastName,bk[k].LastName) > 0) {
            char *temp;


            temp =  bk[k].FirstName;
            bk[k].FirstName = bk[i].FirstName;
            bk[i].FirstName = temp;


            temp =  bk[k].LastName;
            bk[k].LastName = bk[i].LastName;
            bk[i].LastName = temp;


            temp =  bk[k].number;
            bk[k].number = bk[i].number;
            bk[i].number = temp;
        }
      }
  }

---

Also, you are only sorting by LastName, your comment indicates you know that :D