Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

How to fix a C Program bug, preventing Multiple File Creations in a single execution?

I am a beginner, working in VS, in a Microsoft environment, making a To Do List in C. After trying to make the program with the help of Stackoverflow, Chatgpt and a good friend, I manged to do it, although with a bug.

The bug occurs when a User tries to make two files in one run, the program will stop and send me this error message: Exception thrown at 0x00007FF623F51ED9 in To do list.exe: 0xC0000005: Access violation writing location 0x0000000000000008.

The error is pinpointed at this line (*fileArray)[*size] = _strdup(fileName);

If I close the terminal and press continue, it works again, but it will still happen after the User tries to create 2 files in one run.

The second file does get created in the desired directory but it doesn't get an index position in "index_data.txt", this happens to every file that gets the error after getting created.

If you want to recreate the bug just replace my directory with yours and run the program, after running choose 1 and type a name like "list0" afterwards enter and press A to return to choices again, press 1 and write a name like "list1" after pressing enter the bug should happen

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdbool.h>
#include <sys/stat.h>
#include <errno.h>
#include <direct.h>
#include <io.h>

#define DIRECTORY_NAME "C:\\Users\\abood\\OneDrive\\Desktop\\TDL File"
#define INDEX_FILE "index_data.txt"
#define MAX_FILENAME_LENGTH 256


void ensureDirectoryExists(void);
void prependDirectory(char* fullPath, const char* fileName);
void initializeFileArray(char*** fileArray, int* capacity);
void loadIndexes(char*** fileArray, int* size, int* capacity);                          
void saveIndexes(char** fileArray, int size);                                           
void addFileToArray(char*** fileArray, int* size, int* capacity, const char* fileName);   
void freeFileArray(char*** fileArray, int size);
void displayFiles(char** fileArray, int size);
void FcreateList();

char input[100];
int choice = 0;
char listName[MAX_FILENAME_LENGTH];
char line[255];
FILE* fh;
bool keepGoing = true;
char fullPath[MAX_FILENAME_LENGTH];
char** fileArray = NULL;
int size = 0;
int capacity = 0;

int main() {

    ensureDirectoryExists();

    loadIndexes(&fileArray, &size, &capacity);

    do {
        printf("1. Create a new List\n");
        printf("2. Add a new Task\n");
        printf("3. Show your List\n");

        scanf("%d", &choice);
        getchar();

        switch (choice) {
        case 1:
            printf("Type a name for your list\n");
            FcreateList();
            printf("List has been created successfully\n");
            break;
        case 2:
            printf("Blank\n");
            break;
        case 3:
            printf("Blank\n");
            break;
        default:
            printf("Error opening the file\n");
        }

        printf("\nPress A to return to main Menu. Press any other Key To Exit: ");
        scanf(" %c", &input);

        system("cls");
    } while (input[0] == 'A' || input[0] == 'a');
    return 0;
}

void FcreateList() {
    scanf("%s", listName);
    getchar();
    prependDirectory(fullPath, listName);  // Prepend directory to file name
    fh = fopen(fullPath, "w");
    if (fh == NULL) {
        printf("Error creating file.\n");
        return;
    }

    system("cls");
    fclose(fh);

    addFileToArray(&fileArray, &size, &capacity, fullPath);
    printf("File '%s' created and stored at index %d.\n", fullPath, size - 1);

    system("cls");

    // Display all files with their indexes
    displayFiles(fileArray, size);

    // Save indexes to the persistent file
    saveIndexes(fileArray, size);

    // Free allocated memory
    freeFileArray(&fileArray, size);
}

void ensureDirectoryExists(void) {
    if (_access(DIRECTORY_NAME, 0) == -1) { // Check if directory exists
        if (_mkdir(DIRECTORY_NAME) != 0) { // Create the directory if it doesn't exist
            perror("Failed to create directory");
            exit(EXIT_FAILURE);
        }
    }
}

void prependDirectory(char* fullPath, const char* fileName) {
    snprintf(fullPath, MAX_FILENAME_LENGTH, "%s\\%s.txt", DIRECTORY_NAME, fileName);
}

void initializeFileArray(char*** fileArray, int* capacity) {
    *capacity = 2;  // The Array starts with 2
    *fileArray = (char**)malloc(*capacity * sizeof(char*));  // Allocate memory for the array of pointers

    // Check if malloc succeeded
    if (*fileArray == NULL) {
        perror("Failed to allocate memory for file array");
        exit(EXIT_FAILURE);
    }
}

void loadIndexes(char*** fileArray, int* size, int* capacity) {
    char indexPath[MAX_FILENAME_LENGTH];
    prependDirectory(indexPath, INDEX_FILE);  // Prepend directory to index file name

    FILE* file = fopen(indexPath, "r");
    if (!file) {
        printf("No existing index file found. Initializing empty file array.\n");
        initializeFileArray(fileArray, capacity);  // Ensure the file array is initialized
        *size = 0;
        return;
    }

    char buffer[MAX_FILENAME_LENGTH];
    *size = 0;
    initializeFileArray(fileArray, capacity);

    while (fgets(buffer, sizeof(buffer), file)) {
        buffer[strcspn(buffer, "\n")] = '\0';
        addFileToArray(fileArray, size, capacity, buffer);
    }

    fclose(file);
}

void saveIndexes(char** fileArray, int size) {
    char indexPath[MAX_FILENAME_LENGTH];
    prependDirectory(indexPath, INDEX_FILE);  // Prepend directory to index file name

    FILE* file = fopen(indexPath, "w");
    if (!file) {
        perror("Failed to open index file for writing");
        return;
    }

    for (int i = 0; i < size; i++) {
        fprintf(file, "%s\n", fileArray[i]);
    }

    fclose(file);
}

void addFileToArray(char*** fileArray, int* size, int* capacity, const char* fileName) {
    if (fileName == NULL || strlen(fileName) == 0) {
        printf("Error: Invalid file name.\n");
        return;
    }

    // Check if size is smaller or equals to the current capacity of elements
    if (*size >= *capacity) {
        *capacity *= 2; // double the capacity number
        char** newArray = (char**)realloc(*fileArray, *capacity * sizeof(char*));  // Resize the array

        // Check if realloc succeeded
        if (newArray == NULL) {
            perror("Failed to reallocate memory");
            exit(EXIT_FAILURE);
        }

        *fileArray = newArray;
    }

    // Attempt to duplicate the file name and store it in the array
    (*fileArray)[*size] = _strdup(fileName);
    if ((*fileArray)[*size] == NULL) {
        perror("Failed to duplicate file name");
        exit(EXIT_FAILURE);
    }

    (*size)++;  // Increase the size of the file array
}
void displayFiles(char** fileArray, int size) {
    printf("\nFiles and their indexes:\n");
    if (size == 0) {
        printf("No files available.\n");
        return;
    }
    for (int i = 0; i < size; i++) {
        printf("Index %d: %s\n", i, fileArray[i]);
    }
}

void freeFileArray(char*** fileArray, int size) {
    if (*fileArray) {
        for (int i = 0; i < size; i++) {
            free((*fileArray)[i]);
        }
        free(*fileArray);
        *fileArray = NULL;
    }
}

I tried to name the files like: file0, file1, list242, listnew, exp12 etc.. it doesn't change anything. I tried naming the file: list 1, the file gets created but the program exits afterwards. I tried removing the created files and keeping the index_data.txt, but the same bug occurs. Tried using the debugger, to understand where the problem lays, didn't get me anywhere.

My friend codes mainly in C++ and knows a little about C, so he is unfortunately clueless about the problem.

Chatgpt keeps saying it's a memory allocation problem and keeps rewriting the code but it never helped. I read the code a couple of times and I am not really sure how to solve it.

like image 644
Anon26 Avatar asked Sep 01 '25 03:09

Anon26


2 Answers

initializeFileArray() is called from within loadIndexes(), which is called just once, at the start of the program.

When processing the first file, you call freeFileArray() at the end of FcreateList(), which sets *fileArray to NULL. Then initializeFileArray() is not called again before the next call to FcreateList() to process the next file, and so you call addFileToArray with *fileArray == NULL. As such, attempting to write to (*fileArray)[*size] crashes the program.

(Side note: it's very confusing to have a global variable called fileArray, and have all your functions take an argument with the same name that is a pointer to the global variable.)

like image 168
Nate Eldredge Avatar answered Sep 02 '25 17:09

Nate Eldredge


[As Nate pointed out] it is better to use a struct instead of three separate variables.

So, I refactored the code to use one (e.g. struct arr below).

FcreateList and loadIndexes used the global fileArray but addFileToArray used an argument for this.

If we were creating a [reusable] library for this, we'd want to avoid the [inconsistent] usage of global variables.

Now, most functions take a single argument (a pointer to the struct) instead of separate arguments for array, size, and capacity.

Note that some functions [that would change the array] have to have indirect pointers.

With the struct, all functions can take consistent arguments.

Although this was intended as [merely] a first cut before debugging the [real] issue, in so doing, it seems to have eliminated your bug:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdbool.h>
#include <sys/stat.h>
#include <errno.h>

#ifdef __linux__
#define DIRECTORY_NAME "TDL_File"
#include <unistd.h>
#define _access access
#define _strdup strdup
#define _mkdir(_file)   mkdir(_file,0755)
#define CLEAR   do { } while (0)
#else
#include <direct.h>
#include <io.h>
#define DIRECTORY_NAME "C:\\Users\\abood\\OneDrive\\Desktop\\TDL File"
#define CLEAR   system("cls")
#endif

#if 0
#define INDEX_FILE "index_data.txt"
#else
#define INDEX_FILE "index_data"
#endif
#define MAX_FILENAME_LENGTH 256

char input[100];
int choice = 0;
char listName[MAX_FILENAME_LENGTH];
char line[255];
FILE *fh;
bool keepGoing = true;
char fullPath[MAX_FILENAME_LENGTH];

// NOTE/BUG: [inconsistently] using globals like this is problematic (a source
// of bugs)
#if 0
char **fileArray = NULL;
int size = 0;
int capacity = 0;
#else
struct arr {
    char **data;
    int size;
    int capacity;
};
#endif

void
displayFiles(const struct arr *arr)
{
    printf("\nFiles and their indexes:\n");
    if (arr->size == 0) {
        printf("No files available.\n");
        return;
    }
    for (int i = 0; i < arr->size; i++) {
        printf("Index %d: %s\n", i, arr->data[i]);
    }
}

void
addFileToArray(struct arr *arr, const char *fileName)
{
    if (fileName == NULL || strlen(fileName) == 0) {
        printf("Error: Invalid file name.\n");
        return;
    }

    // Check if size is smaller or equals to the current capacity of elements
    if (arr->size >= arr->capacity) {
        // double the capacity number
        arr->capacity *= 2;

        // Resize the array
        char **newArray = realloc(arr->data, arr->capacity * sizeof(char *));

        // Check if realloc succeeded
        if (newArray == NULL) {
            perror("Failed to reallocate memory");
            exit(EXIT_FAILURE);
        }

        arr->data = newArray;
    }

    // Attempt to duplicate the file name and store it in the array
    arr->data[arr->size] = _strdup(fileName);
    if (arr->data[arr->size] == NULL) {
        perror("Failed to duplicate file name");
        exit(EXIT_FAILURE);
    }

    // Increase the size of the file array
    arr->size++;
}

void
freeFileArray(struct arr *arr)
{
    if (arr->data != NULL) {
        for (int i = 0; i < arr->size; i++) {
            free(arr->data[i]);
        }

        free(arr->data);
        arr->data = NULL;

#if 1
        arr->size = 0;
        arr->capacity = 0;
#endif
    }
}

void
prependDirectory(char *fullPath, const char *fileName)
{
    snprintf(fullPath, MAX_FILENAME_LENGTH, "%s/%s.txt",
        DIRECTORY_NAME, fileName);
}

void
saveIndexes(const struct arr *arr)
{
    char indexPath[MAX_FILENAME_LENGTH];

    // Prepend directory to index file name
    prependDirectory(indexPath, INDEX_FILE);

    FILE *file = fopen(indexPath, "w");

    if (!file) {
        perror("Failed to open index file for writing");
        return;
    }

    for (int i = 0; i < arr->size; i++) {
        fprintf(file, "%s\n", arr->data[i]);
    }

    fclose(file);
}

void
FcreateList(struct arr *arr)
{
    scanf("%s", listName);
    getchar();

    // Prepend directory to file name
    prependDirectory(fullPath, listName);

    fh = fopen(fullPath, "w");
    if (fh == NULL) {
        printf("Error creating file '%s' -- %s\n",fullPath,strerror(errno));
        return;
    }

    CLEAR;
    fclose(fh);

    addFileToArray(arr, fullPath);
    printf("File '%s' created and stored at index %d.\n",
        fullPath, arr->size - 1);

    CLEAR;

    // Display all files with their indexes
    displayFiles(arr);

    // Save indexes to the persistent file
    saveIndexes(arr);

    // Free allocated memory
    freeFileArray(arr);
}

void
ensureDirectoryExists(void)
{

    // Check if directory exists
    if (_access(DIRECTORY_NAME, 0) == -1) {
        // Create the directory if it doesn't exist
        if (_mkdir(DIRECTORY_NAME) != 0) {
            perror("Failed to create directory");
            exit(EXIT_FAILURE);
        }
    }
}

void
initializeFileArray(struct arr *arr)
{
    // The Array starts with 2
    arr->capacity = 2;

    // Allocate memory for the array of pointers
    arr->data = malloc(arr->capacity * sizeof(char *));

    // Check if malloc succeeded
    if (arr->data == NULL) {
        perror("Failed to allocate memory for file array");
        exit(EXIT_FAILURE);
    }

// NOTE/FIX: don't rely on caller doing this
#if 1
    arr->size = 0;
#endif
}

void
loadIndexes(struct arr *arr)
{
    char indexPath[MAX_FILENAME_LENGTH];

    // Prepend directory to index file name
    prependDirectory(indexPath, INDEX_FILE);

    FILE *file = fopen(indexPath, "r");

    if (!file) {
        printf("No existing index file found. Initializing empty file array.\n");
        // Ensure the file array is initialized
        initializeFileArray(arr);
#if 0
        arr->size = 0;
#endif
        return;
    }

    char buffer[MAX_FILENAME_LENGTH];

#if 0
    arr->size = 0;
#endif
    initializeFileArray(arr);

    while (fgets(buffer, sizeof(buffer), file)) {
        buffer[strcspn(buffer, "\n")] = '\0';
        addFileToArray(arr, buffer);
    }

    fclose(file);
}

int
main(void)
{

// NOTE/FIX: eliminate [critical] globals
#if 1
    static struct arr fileArray;
    struct arr *arr = &fileArray;
#endif

    ensureDirectoryExists();

    loadIndexes(arr);

    do {
        printf("1. Create a new List\n");
        printf("2. Add a new Task\n");
        printf("3. Show your List\n");

        scanf("%d", &choice);
        getchar();

        switch (choice) {
        case 1:
            printf("Type a name for your list\n");
            FcreateList(arr);
            printf("List has been created successfully\n");
            break;
        case 2:
            printf("Blank\n");
            break;
        case 3:
            printf("Blank\n");
            break;
        default:
            printf("Error opening the file\n");
            break;
        }

        printf("\nPress A to return to main Menu. Press any other Key To Exit: ");
#if 0
        scanf(" %c", &input);
#else
        scanf(" %c", input);
#endif

        CLEAR;
    } while (input[0] == 'A' || input[0] == 'a');

    return 0;
}

In the code above, I've used cpp conditionals to denote old vs. new code:

#if 0
// old code
#else
// new code
#endif

#if 1
// new code
#endif

Note: this can be cleaned up by running the file through unifdef -k

like image 35
Craig Estey Avatar answered Sep 02 '25 15:09

Craig Estey