Can struct member pointers overflow?

Question

I am implementing a music scale database via a double pointer construct, but I keep running into segfaults and overflow errors when malloc'ing space for items inside the database and I do not understand why.

This is a conceptual issue, the error is incompatible with my understanding of how pointers work and I have been unable to find a definitive answer online. The issue seems to stem from the line

db->entry[db_idx] = malloc(sizeof(struct scale_t)); // potiential overflow here??
db->entry[db_idx]->scale = circularlist_create();

But I do not understand how this can be since, db->entry[db_idx] is type struct scale_t* and malloc is returning a pointer of the appropriate type. The amount malloc'd shouldn't really matter since I am writing a pointer value to db->entry[db_idx]

Anyway, here is a link to the #include "CircularLinkedList.h" header and implementation files. https://gist.github.com/jstaursky/58d4466eb232e90580e1011bf5a7e641 https://gist.github.com/jstaursky/84cf9ba2f870da0807faa454f20c36e9

the scale.list file https://gist.github.com/jstaursky/24baeaf2a922a081f0a919d31ed638df

The directory structure looks like this

src
 - main.c
 - CircularLinkedList.h
 - CircularLinkedList.c
 - conf/
    - scale.list

Added the gists to try and keep the question as compact as possible.

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

#include "CircularLinkedList.h" // See linked GitHub gists.

struct scale_t {
    char* name;
    struct node_t* scale;
    int num_notes;
};

struct database_t {
    struct scale_t** entry;
    int size;
};

struct database_t *build_database(FILE *);
char *fgetline(FILE *stream);

int main(int argc, char *argv[]) 
{
    FILE *configfp = fopen("conf/scale.list", "r");
    struct database_t *scaledatabase = build_database(configfp);

    for (int i = 0; i < scaledatabase->size; ++i) {
        circularlist_traverse(scaledatabase->entry[i]->scale, circularlist_print);
    }
}

struct database_t *build_database(FILE *fp)
{
    struct database_t *db = malloc(sizeof(struct database_t));
    db->entry = malloc(sizeof(struct scale_t *));

    int db_idx = 0;

    for (char *line; (line = fgetline(fp)); ++db_idx) {
        db->entry[db_idx] = malloc(sizeof(struct scale_t)); // potiential overflow here??
        db->entry[db_idx]->scale = circularlist_create();

        char *rest = line;
        db->entry[db_idx]->name = strtok_r(line, ",", &rest);

        while (isspace(*rest))
            ++rest;

        char *interval;
        int note_count = 0;
        while ((interval = strtok_r(NULL, "-", &rest))) {
            circularlist_insert(&db->entry[db_idx]->scale, interval);
            ++note_count;
        }
        db->entry[db_idx]->num_notes = note_count;
    }
    db->size = db_idx;

    return db;
}

char*
fgetLine(FILE *stream)
{
    const size_t chunk = 128;
    size_t max = chunk;
    /* Preliminary check */

    if (!stream || feof(stream))
        return NULL;

    char *buffer = (char *)malloc(chunk * sizeof(char));

    if (!buffer) {
        perror("Unable to allocate space");
        return NULL;
    }
    char *ptr = buffer;

    int c; /* fgetc returns int. Comparing EOF w/ char may cause issues. */
    while ( (c = fgetc(stream)) != EOF && (*ptr = c) != '\n')
    {
        ++ptr;
        size_t offset = ptr - buffer;

        if (offset >= max) {
            max += chunk;
            char *tmp = realloc(buffer, max);

            if (!tmp) {
                free(buffer);
                return NULL;
            }
            buffer = tmp;
            ptr = tmp + offset;
        }
    }
    *ptr = '\0';
    return buffer;
}

Answer 1

The problem in your code isn't with the struct of the entry, but in the struct of database.

From your code, you want the database to have an array of entries, but its definition and use doesn't do it as it is implemented right now.

You want the database to have a variable length size of array as its first argument (as I understand from your code), but the definition and use of it isn't right.

When you are allocating the memory for the database:

struct database_t *db = malloc(sizeof(struct database_t));

would allocate the size of the sturct, which would be the size of a pointer (entry) plus the size of int (size), which means that the entry pointer is still only a pointer, and not an array.

To fix this problem, there are couple of things you can do:

Save array with maximum length

You can change the definition of the struct to be something like this:

struct database_t {
    struct scale_t* entry[MAX_LENGTH];
    int size;
};

This would make your first malloc operation to work and allocate all the memory you need.

The disadvantages of this solution is that it would use constant length of memory for all the entry arrays, and you would be restricted by the maximum length of a database.

Realloc the memory for each new entry

Another solution for the problem is to allocate the memory for the array by yourself.

You should realloc the memory of the array in each run of the for loop, each time increasing the size you use, to save all the memory.

The disadvantages of this solution is that it makes much more allocations in the program, which would make the initialization process take more run time, and be more complicated.

The new code of the function init function should look something like this:

struct database_t *build_database(FILE *fp)
{
    struct database_t *db = malloc(sizeof(struct database_t));

    int db_idx = 0;
    /* Ensure that the value starts from NULL. */
    db->entry = NULL;

    for (char *line; (line = fgetline(fp)); ++db_idx) {
        /* Realloc the memory, adding the new needed memory for the new entry. */
        db->entry = realloc(db->entry, sizeof((struct scale_t *) * (db_idx + 1)));
        db->entry[db_idx] = malloc(sizeof(struct scale_t));
        db->entry[db_idx]->scale = circularlist_create();

        char *rest = line;
        db->entry[db_idx]->name = strtok_r(line, ",", &rest);

        while (isspace(*rest))
            ++rest;

        char *interval;
        int note_count = 0;
        while ((interval = strtok_r(NULL, "-", &rest))) {
            circularlist_insert(&db->entry[db_idx]->scale, interval);
            ++note_count;
        }
        db->entry[db_idx]->num_notes = note_count;
    }
    db->size = db_idx;

    return db;
}