Segmentation Fault - Dynamic Memory Allocation

Question

I have been having this issue surround some code I have been given to work on. This issue is somewhere in the menu_add_book function or read_line function, it compiles with no issues yet when I run it I can enter the first option okay using an input file yet when it comes to the title a segmentation fault arises . I am fairly sure this is something to do with allocating memory but for the life of me cannot work out what, if someone could help explain it to me you would be a life saver. my code is as follows

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

#define MAX_TITLE_LENGTH  100
#define MAX_AUTHOR_LENGTH 100

/* Book structure */

struct Book
{
   /* Book details */
   char title[MAX_TITLE_LENGTH+1];   /* name string */
   char author[MAX_AUTHOR_LENGTH+1]; /* job string */
   int  year;                        /* year of publication */

   /* pointers to left and right branches pointing down to next level in
      the binary tree */
   struct Book *left, *right;
};

/* tree of books, initialized to NULL. */
static struct Book *book_tree = NULL;

/* read_line():
 *
 * Read line of characters from file pointer "pf", copying the characters
 * into the "line" string, up to a maximum of "max_length" characters, plus
 * one for the string termination character '\0'. Reading stops upon
 * encountering the end-of-line character '\n', for which '\0' is substituted
 * in the string. If the end of file character EOF is reached before the end
 * of the line, the failure condition (-1) is returned. If the line is longer
 * than the maximum length "max_length" of the string, the extra characters
 * are read but ignored. Success is returned (0) on successfully reading
 * a line.
 */
static int read_line ( FILE *pf, char *line, int max_length )
{
   int i;
   char ch;

   /* initialize index to string character */
   i = 0;

   /* read to end of line, filling in characters in string up to its
      maximum length, and ignoring the rest, if any */
   for(;;)
   {
      /* read next character */
      ch = fgetc(pf);

      /* check for end of file error */
      if ( ch == EOF )
     return -1;

      /* check for end of line */
      if ( ch == '\n' )
      {
     /* terminate string and return */
     line[i] = '\0';
     return 0;
      }

      /* fill character in string if it is not already full*/
      if ( i < max_length )
     line[i++] = ch;
   }

   /* the program should never reach here */
   return -1;
}

/* read_string():
 *
 * Reads a line from the input file pointer "pf", starting with the "prefix"
 * string, and filling the string "string" with the remainder of the contents
 * of the line. If the start of the line does not match the "prefix" string,
 * the error condition (-1) is returned. Having read the prefix string,
 * read_string() calls read_line() to read the remainder of the line into
 * "string", up to a maximum length "max_length", and returns the result.
 */
static int read_string ( FILE *pf,
             char *prefix, char *string, int max_length )
{
   int i;

   /* read prefix string */
   for ( i = 0; i < strlen(prefix); i++ )
      if ( fgetc(pf) != prefix[i] )
     /* file input doesn't match prefix */
     return -1;

   /* read remaining part of line of input into string */
   return ( read_line ( pf, string, max_length ) );
}

/* menu_add_book():
 *
 * Add new book to database
 */
static void menu_add_book(void)
{
FILE *pf = NULL;
char *line = NULL; 
int max_length = 100;
int year;
struct Book *new = NULL;
new = (struct Book *) malloc ( sizeof(struct Book) );
fprintf(stderr,"\nEnter Title:");
read_line ( pf, line, max_length );
fprintf(stderr,"\nEnter Author:");
read_line ( pf, line, max_length );
fprintf(stderr,"\nEnter Year:\n");
scanf( "%i", &year);
}

/* menu_print_database():
 *
 * Print database of books to standard output in alphabetical order of title.
 */
static void menu_print_database(void)
{
   /* fill in the code here in part 1, and add any extra functions you need */
}       

/* menu_get_book_details():
 *
 * Get details of book from database.
 */
static void menu_get_book_details(void)
{
   /* fill in the code here in part 2, and add any extra functions you need */
}

/* menu_delete_book():
 *
 * Delete new book from database.
 */
static void menu_delete_book(void)
{
   /* fill in the code here in part 2, and add any extra functions you need */

}

/* read file containing database of books */
static void read_book_database ( char *file_name )
{
   /* fill in the code here in part 3, and add any extra functions you need */
}

/* get_tree_depth():
 *
 * Recursive function to compute the number of levels in a binary tree.
 */
static int get_tree_depth ( struct Book *book, int level )
{
   int level1, level2;

   /* return with the current level if we've reached the bottom of this
      branch */
   if ( book == NULL ) return level;

   /* we need to go to the next level down */
   level++;

   /* count the number of levels down both branches */
   level1 = get_tree_depth ( book->left,  level );
   level2 = get_tree_depth ( book->right, level );

   /* return the depth of the deepest branch */
   if ( level1 > level2 ) return level1;
   else return level2;
}

/* menu_print_tree():
 *
 * Print tree to standard output. You can use this function to print out the
 * tree structure for debugging purposes. It is also used by the testing
 * software to check that the tree is being built correctly.
 *
 * The first letter of the title of each book is printed.
 */
static void menu_print_tree(void)
{
   int no_levels, level, size, i, j, k;
   struct Book **row;

   /* find level of lowest node on the tree */
   no_levels = get_tree_depth ( book_tree, 0 );

   /* abort if database is empty */
   if ( no_levels == 0 ) return;

   /* compute initial indentation */
   assert ( no_levels < 31 );

   row = (struct Book **) malloc((1 << (no_levels-1))*sizeof(struct Book *));
   row[0] = book_tree;
   printf ( "\n" );
   for ( size = 1, level = 0; level < no_levels; level++, size *= 2 )
   {
      /* print books at this level */
      for ( i = 0; i < size; i++ )
      {
     if ( i == 0 )
        for ( j = (1 << (no_levels - level - 1)) - 2; j >= 0; j-- )
           printf ( " " );
     else
        for ( j = (1 << (no_levels - level)) - 2; j >= 0; j-- )
           printf ( " " );

     if ( row[i] == NULL )
        printf ( " " );
         else
        printf ( "%c", row[i]->title[0] );
      }

      printf ( "\n" );

      if ( level != no_levels-1 )
      {
     /* print connecting branches */
     for ( k = 0; k < ((1 << (no_levels - level - 2)) - 1); k++ )
     {
        for ( i = 0; i < size; i++ )
        {
           if ( i == 0 )
          for ( j = (1 << (no_levels - level - 1))-3-k; j >= 0; j-- )
             printf ( " " );
           else
          for ( j = (1 << (no_levels - level)) - 4 - 2*k; j >= 0; j-- )
             printf ( " " );

           if ( row[i] == NULL || row[i]->left == NULL )
          printf ( " " );
           else
          printf ( "/" );

           for ( j = 0; j < 2*k+1; j++ )
          printf ( " " );

           if ( row[i] == NULL || row[i]->right == NULL )
          printf ( " " );
           else
          printf ( "\\" );
        }

        printf ( "\n" );
     }

     /* adjust row of books */
     for ( i = size-1; i >= 0; i-- )
     {
        row[2*i+1] = (row[i] == NULL) ? NULL : row[i]->right;
        row[2*i]   = (row[i] == NULL) ? NULL : row[i]->left;
     }
      }
   }

   free(row);
}       

/* codes for menu */
#define ADD_CODE     0
#define DETAILS_CODE 1
#define DELETE_CODE  2
#define PRINT_CODE   3
#define TREE_CODE    4
#define EXIT_CODE    5

int main ( int argc, char *argv[] )
{
   /* check arguments */
   if ( argc != 1 && argc != 2 )
   {
      fprintf ( stderr, "Usage: %s [<database-file>]\n", argv[0] );
      exit(-1);
   }

   /* read database file if provided, or start with empty database */
   if ( argc == 2 )
      read_book_database ( argv[1] );

   for(;;)
   {
      int choice, result;
      char line[301];             

      /* print menu to standard error */
      fprintf ( stderr, "\nOptions:\n" );
      fprintf ( stderr, "%d: Add new book to database\n",      ADD_CODE );
      fprintf ( stderr, "%d: Get details of book\n",       DETAILS_CODE );
      fprintf ( stderr, "%d: Delete book from database\n",  DELETE_CODE );
      fprintf ( stderr, "%d: Print database to screen\n",    PRINT_CODE );
      fprintf ( stderr, "%d: Print tree\n",                   TREE_CODE );
      fprintf ( stderr, "%d: Exit database program\n",        EXIT_CODE );
      fprintf ( stderr, "\nEnter option: " );

      if ( read_line ( stdin, line, 300 ) != 0 ) continue;

      result = sscanf ( line, "%d", &choice );
      if ( result != 1 )
      {
     fprintf ( stderr, "corrupted menu choice\n" );
     continue;
      }

      switch ( choice )
      {
         case ADD_CODE: /* add book to database */
     menu_add_book();
     break;

         case DETAILS_CODE: /* get book details from database */
     menu_get_book_details();
     break;

         case DELETE_CODE: /* delete book from database */
     menu_delete_book();
     break;

         case PRINT_CODE: /* print database contents to screen
                 (standard output) */
     menu_print_database();
     break;

         case TREE_CODE: /* print tree to screen (standard output) */
     menu_print_tree();
     break;

     /* exit */
         case EXIT_CODE:
     break;

         default:
     fprintf ( stderr, "illegal choice %d\n", choice );
     break;
      }

      /* check for exit menu choice */
      if ( choice == EXIT_CODE )
     break;
   }

   return 0;   
}

Answer 1

You have initialized line as:

char *line = NULL; 

but you have not allocated any memory for it.

Add

line = malloc(max_length+1);

before you call read_line.

Answer 2

You have char* line = NULL and then you allocate memory dynamically for the new.

However, you pass the line as parameter in the function read_line and then you try to access its i-th element, with [] operator and thus, you get a seg. fault.

In order to fix it, you can do this:

char *line = NULL;
int max_length = 100;
line = malloc( (max_length + 1) * sizeof(char) ); // +1 for the null terminator

Also, printing the menu in stderr is not something usual. stderr is the stream for errors. Menu is not an error.

I would write, instead of this:

fprintf ( stderr, "\nOptions:\n" );

this

fprintf ( stdout, "\nOptions:\n" ); // or use printf

You may also want to read: Why not casting the result of malloc in C?

Another, more important logical error, is near the error that caused your segmentation fault.

You do not initialize file pointer pf, but then you expect to read from it in read_line.

FILE *pf = NULL;                  // the file pointer
char *line = NULL;                // fix the initial logical error
int max_length = 100;
line = malloc(max_length + 1);
int year;
struct Book *new = NULL;
new = (struct Book *) malloc ( sizeof(struct Book) );
fprintf(stderr,"\nEnter Title:");
read_line ( pf, line, max_length ); // Here, you pass pf uninitialized

and then in read_line(), you do:

char ch;       // you may want to change that in: int char;
ch = fgetc(pf);

So, where are is the file pointer set to? Nowhere!

And also, you named your variable pf, but it is a file pointer, so fp, would be better, IMHO.

Maybe there are other errors too, but I think that is enough for now.