I wrote a program that uses structures and dynamic memory allocation. I would like to get help understanding why if I press 4 (which means exit the menu) I get the error:
HEAP CORRUPTION DETECTED: after Normal block (#76) at 0x00845EE8.
It is linked to the function deletelist(). I really don't understand why it happens and how to fix this problem.
This is the code:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define PRO_OP 1
#define CON_OP 2
#define PRINT_OP 3
#define EXIT_OP 4
#define STR_LEN 50
#define MAX_LIST_LENGTH 10
typedef struct reasonList
{
char* listName;
char* reasons[MAX_LIST_LENGTH];
int numReasons;
} reasonList;
void initList(reasonList* list, char* name);
void addReason(reasonList* list);
void printList(reasonList list);
int menu(void);
void myFgets(char str[], int n);
void deleteList(reasonList* list);
int main(void)
{
char dilemma[STR_LEN] = { 0 };
int op = 0;
reasonList proList;
initList(&proList, "PRO");
reasonList conList;
initList(&conList, "CON");
printf("What is your dilemma?\n");
myFgets(dilemma, STR_LEN);
while (op != EXIT_OP)
{
op = menu();
switch (op)
{
case(PRO_OP):
addReason(&proList);
break;
case(CON_OP):
addReason(&conList);
break;
case(PRINT_OP):
printf("Your dilemma:\n");
printf("%s\n\n", dilemma);
printList(proList);
printList(conList);
break;
case(EXIT_OP):
deleteList(&proList);
deleteList(&conList);
break;
}
}
printf("Good luck!\n");
getchar();
return 0;
}
/*
Function will initialize a reason list
input: the list to init, and its name
output: none
*/
void initList(reasonList* list, char* listName)
{
list->listName = (char*) malloc(sizeof(char));
strcpy(list->listName, listName);//equal to PRO or CON
for (int i = 0; i < MAX_LIST_LENGTH; i++)
{
list->reasons[i] = (char*)malloc(sizeof(char) * STR_LEN);
if (list->reasons[i] == NULL)
{
printf("no memmory left\n");
exit(1);
}
strcpy(list->reasons[i], "");
}
list->numReasons = 0;
}
/*
Function will add a reason to the list
input: the list to add to and its name
output: none
*/
//
void addReason(reasonList* list)
{
char* newReason = (char*)malloc(sizeof(char) * STR_LEN);
printf("Enter a reason to add to the list %s:\n", list->listName);
myFgets(newReason, STR_LEN);
if (list->numReasons >= MAX_LIST_LENGTH)//if no memory left
{
//free(newReason);
return;
}
free(list->reasons[list->numReasons]);
list->reasons[list->numReasons] = newReason;
list->numReasons++;
list->reasons[list->numReasons] = NULL;
}
/*
Function will print a list of reasons
input: the list
output: none
*/
void printList(reasonList list)
{
printf("list %s\n--------\n", list.listName);
for (int i = 0; i < list.numReasons; i++)
{
printf("%s", list.reasons[i]);
printf("\n");
}
printf("\n");
}
/*
Function shows menu and returns user's choice
input: none
output: user's choice
*/
int menu(void)
{
int op = 0;
printf("Choose option:\n");
printf("%d - Add PRO reason\n", PRO_OP);
printf("%d - Add CON reason\n", CON_OP);
printf("%d - Print reasons\n", PRINT_OP);
printf("%d - Exit\n", EXIT_OP);
scanf("%d", &op);
while (op < PRO_OP || op > EXIT_OP)
{
printf("Invalid option. Try again: ");
scanf("%d", &op);
}
getchar(); // clean buffer
return op;
}
/*
Function will delete a list
input: the list to delete
output: none
*/
void deleteList(reasonList* list)
{
for (int i = 0; i < list->numReasons; i++)
{
free(list->reasons[i]);
list->reasons[i] = NULL; // set pointer to NULL
}
free(list->listName);
list->listName = NULL; // set pointer to NULL
}
/*
Function will perform the fgets command and also remove the newline
that might be at the end of the string - a known issue with fgets.
input: the buffer to read into, the number of chars to read
*/
void myFgets(char str[], int n)
{
fgets(str, n, stdin);
str[strcspn(str, "\n")] = 0;
}
I was not able to reproduce the segfault but valgrind complained about
initList():malloc(sizeof(char))which allocates 1 bytes (the\0for an empty string) while you needstrlen(listName) + 1.Unrelated, to eliminate the memory leak, observe that you in
initList()allocate space forMAX_LIST_LENGTHreasonsbut indeleteList()you only freelist->numReasons. Suggest you just leave it toaddReasons()to allocate space:If you want to be able to delete (and reuse)
reasons[i]then ininitList()you want to initializelist->reasons[i] = NULLand I suggest (for symmetry) create adeleteReason()function which you then use indeleteList().Another valid design (as your
reasonsare fixed size) is to allocate all thereasonsininitList()(like you had). InaddReaon()reuse the allocated string (nomalloc()orfree()). IndeleteList()free allMAX_LIST_LENGTHreasons.It's a good idea to check the return value from
malloc().Don't cast the
void *frommalloc().Prefer passing a variable opposed to a type to
sizeof. In the above instead ofsizeof(char)usesizeof *list->listNameor sizeof *listName; that saidsize(char)is defined as1` so I always leave it out.