Calling free() sometimes causes program to crash

Question

The code below ask the user to input 10 pairs of artist and titles which can be up to 30 characters long. Everything seems to work fine with allocating the space and printing the data back out. The problem only occurs when I try to free the memory at then end and then only if one of the elements is 4 or more characters long. I suspect I am not allocating the memory correctly but I just can't see it.

// Songs.cpp : This file contains the 'main' function. Program execution begins and ends there.
//
// Experimenting with pointers, structures and dynamic allocation of memory
//
#ifdef _MSC_VER
#define _CRT_SECURE_NO_WARNINGS
#endif


#include <iostream>
#include <stdio.h>

struct songInfo
{
    char* pArtist;      // char pointer for Artist data
    char* pTitle;       // char pointer for Title data
};

// function prototype declarations
void getSongInfo(struct songInfo *songData, char *Artist, char *Title);
void printSongInfo(songInfo *songData);


int main()
{
    struct  songInfo songData[10];      // setup array of 10 elements of the structure SongInfo
    char    sArtist[31];
    char    sTitle[31];

    // prompt user for the artist and title 10 times once for each array element
    for (int i = 0; i < 10; i++) {
        printf("Artist %i: ", i + 1);
        fgets(sArtist, 31, stdin);
        strtok(sArtist, "\n");          // trim out return character
        printf("Title  %i: ", i + 1);
        fgets(sTitle, 31, stdin);
        strtok(sTitle, "\n");           // trim out return character
        getSongInfo(&songData[i], sArtist, sTitle); // allocates the memory and stores the data into the pointer location
    }

    printSongInfo(songData);    // printout the song data stored in the array

    // free up the allocated memory space
    for (int i = 0; i < 10; ++i) {
        free(songData[i].pArtist);
        free(songData[i].pTitle);
}
    return 0;
}

void getSongInfo(struct songInfo *songData, char *Artist, char *Title) {
    songData->pArtist = (char*)malloc(sizeof(Artist) + 1);  // Allocate enough memory to hold the string and the null terminator
    songData->pTitle = (char*)malloc(sizeof(Title) + 1);
    strcpy(songData->pArtist, Artist);                                  // Copy the data into the allocated memory location
    strcpy(songData->pTitle, Title);
}

void printSongInfo(songInfo *songData) {
    printf("\n%-35s %-35s\n", "Artist", "Title");
    printf("%-35s %-35s\n", "-----------------------------------", "-----------------------------------");
        for (int i = 0; i < 10; i++) {      // iterate through the array of elements
        printf("%-35s %-35s\n", songData[i].pArtist, songData[i].pTitle);
    }
}

Answer 1

It's not free() call that is invalid, it's malloc.

If you'd print out sizeof(Artist) + 1, you'd likely get either 5 or 9 (depending on your computer architecture). And the same for Title. You check the size of pointer on your machine, which is constant, not the size of array you received.

Undefined Behvaiour means your code may do anything, including "working for now, but will break later at a correct place". You invoke UB by calling strcpy, which tries to copy data into buffer too short to contain the whole string.

You have to pass the size of array to function or calculate it using strlen inside function (and pray that the string is actually null-terminated).

void getSongInfo(struct songInfo *songData, char *Artist, char *Title) {
    songData->pArtist = (char*)malloc(strlen(Artist) + 1);  // Allocate enough memory to hold the string and the null terminator
    songData->pTitle = (char*)malloc(strlen(Title) + 1);
    strcpy(songData->pArtist, Artist);                                  // Copy the data into the allocated memory location
    strcpy(songData->pTitle, Title);
}