Getting a segfault using realloc

Question

I am trying to solve the following question that recently appeared in my friends' university exams (the exams are now over and papers are released).

Develop a C program to manage a playlist of songs, where each song has a title and a duration (in seconds). Use two separate dynamically allocated arrays to store the titles and durations. Implement user-defined functions to add, remove, update, and display the playlist information using pointer arithmetic. [10 marks]

I made the following attempt:

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

const size_t MAX_NAME_LENGTH = 20;
size_t number_of_songs = 0;

char* names_of_songs;
int* durations;

void add(char* name_of_song, int duration) {
    number_of_songs++;

    names_of_songs = realloc(name_of_song, MAX_NAME_LENGTH * number_of_songs);
    durations = realloc(durations, number_of_songs * sizeof(int));

    int start_location = ((number_of_songs - 1) * MAX_NAME_LENGTH) + 1;
    char* name_location = &name_of_song[start_location];

    strncpy(name_location, name_of_song, MAX_NAME_LENGTH);

    names_of_songs[start_location + strlen(name_of_song) + 1] = 0;
    durations[number_of_songs - 1] = duration;
}

void display() {
    for (size_t i = 0; i < number_of_songs; i++) {
        int position_in_name = 0;
        int start_location = (i * MAX_NAME_LENGTH) + 1;
        char character = 1;

        printf("Name: ");
        while (character != 0) {
            character = names_of_songs[start_location++];
            printf("%c", character);
        }

        printf(", Duration: %d sec\n", durations[i]);
    }
}

int main(int argc, char const *argv[]) {
    names_of_songs = malloc(1);
    durations = malloc(1);

    display();
    add("Something", 2);
    display();
    add("Something else", 32);
    display();
    add("boring song", 4);
    display();

    free(names_of_songs);
    free(durations);

    return 0;
}

When I run it, I get a segmentation fault. Using my debugger, it seems to happen the first time realloc() is run, when add() tries to adjust for the new songs title. What is happening and how do I fix it?

Additionally, how would I debug something like this?

Answer 1

1. relloc() is used to resize dynamic memory. The easiest fix is to just allow the default 0 initialization for global variables.

2. Assign the return value of realloc() to a temporary, otherwise you leak memory if realloc() fails.

3. Incorrect variable used in the first realloc() name_of_song when you meant names_of_songs.

4. Use symbolic constant instead of a const value. You need the former when allocating memory on the stack, otherwise they will become VLAs.

5. The + 1 in start location is wrong.

6. Removed the display() after each add() as it was confusing (to me).

7. (Not fixed) For names_of_songs consider using an array of char pointers (i.e. char **names_of_songs): names_of_songs[number_of_songs] = strndup(name_of_songs, MAX_NAME_LENGTH) which is easier and you don't have wasted space.

8. (Not fixed) If you really want to use a char array, consider indexing into it with a pointer to an char array (*p)[MAX_NAME_LENGTH] so the compiler scales the pointer for you.

9. display(): It is easier to just print a string instead of each character individually. Then you can combine all the printf() calls into one.

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

#define MAX_NAME_LENGTH 20
size_t number_of_songs;
char* names_of_songs;
int* durations;

void add(char* name_of_song, int duration) {
    char *tmp = realloc(names_of_songs, MAX_NAME_LENGTH * (number_of_songs + 1));
    if(!tmp) {
        printf("malloc failed\n");
        return;
    }
    names_of_songs = tmp;
    strncpy(names_of_songs + number_of_songs * MAX_NAME_LENGTH, name_of_song, MAX_NAME_LENGTH);
    names_of_songs[number_of_songs * MAX_NAME_LENGTH + strlen(name_of_song)] = '\0';

    int *tmp2 = realloc(durations, sizeof *tmp2 * (number_of_songs + 1));
    if(!tmp2) {
        printf("malloc failed\n");
        return;
    }
    durations = tmp2;
    durations[number_of_songs] = duration;
    number_of_songs++;
}

void display() {
    for (size_t i = 0; i < number_of_songs; i++)
        printf("Name: %s, Duration %d sec\n",
            names_of_songs + i * MAX_NAME_LENGTH,
            durations[i]
        );
}

int main() {
    add("Something", 2);
    add("Something else", 32);
    add("boring song", 4);
    display();
    free(names_of_songs);
    free(durations);
}

example output:

Name: Something, Duration: 2 sec
Name: Something else, Duration: 32 sec
Name: boring song, Duration: 4 sec