Using malloc to allocate space to a struct

Question

I'm trying to dynamically allocate memory to a struct using the user's input as the size but every time I do it I get a segerror.

my struct is as follows:

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


// structure that holds the info for a phone record
struct PHONE_RECORD {
    char name[50];
    char birthday[12];
    char phone[15];
} *phonebook;

and the code for dynamically allocating it is here:

int num_space(){
    int num_records;
    struct PHONE_RECORD *phonebook;
    printf("Enter num of records: ");
    scanf("%d", &num_records);
    phonebook = (struct PHONE_RECORD*) malloc(sizeof(struct PHONE_RECORD)*num_records);
    if (phonebook == NULL){ 
        printf("Not enough memory.\n");
        return 1;
    }
    free(phonebook);
    return num_records;
}

The code allows the user to type in a number but then gives me a segerror and exits the program. There are other parts in this project but I have tested them all and they work without issue its only the malloc part that doesn't work. For reference here is my main:

#include <stdio.h>
#include <string.h>
#include "mini4Bphone.c"

extern void addRecord();
extern void findRecords();
extern void listRecords();
extern void loadCSV();
extern void saveCSV();
extern int num_space();

// dispaly the menu
void menu() {
    int choice;

    num_space();

    //display unitl user quits using while loop and execute whatever command user inputs 
    while (1) {
        printf("Phonebook Menu: ");
        printf("(1) Add ");
        printf("(2) Find ");
        printf("(3) List ");
        printf("(4) Quit ");
        printf("> ");
        scanf("%d", &choice);

        switch (choice) {
        case 1:
            addRecord();
            break;
        case 2:
            findRecord();
            break;
        case 3:
            listRecords();
            break;
        case 4:
            return;
        default:
            printf("Invalid choice.\n");
            break;
        }
    }
}

// load tne csv,menu and save the csv after all wanted functions are complete, return 0
int main() {
    loadCSV();
    menu();
    saveCSV();
    return 0;
}

Thanks for any input its appreciated!

I tried using malloc inside and outside a function to no avail. It's supposed to et the user input a number and then allocate the space to the struct. However, every time I tried to run the program I get a segerror.

Can I ask why you allocate the array and then de-allocate it right after? — user253751, Mar 31 '23 at 17:21
The declaration of the structure shouldn't declare a variable as well. — Barmar, Mar 31 '23 at 17:22
[don't cast malloc](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) — Barmar, Mar 31 '23 at 17:22
`num_space()` isn't assigning the global `phonebook` variable. It's creating a local variable, allocating memory for it, then freeing it. If your other functions try to use the global variable it will still be `NULL` and you'll get a segmentation fault. — Barmar, Mar 31 '23 at 17:24
You are including a ".c" file. Only header files should be included. — Chris, Mar 31 '23 at 18:14
It's way more helpful to provide a [mre] instead of 3 incomplete snippets of code. — Allan Wind, Apr 01 '23 at 05:37

Allan Wind · Answer 1 · 2023-04-01T06:07:30.453

The local variable struct PHONE_RECORD *phonebook; shadows the global variable of the same name.
num_space() allocate space then free it. This is pointless. Presumably you want to allocate space for the global variable:

int num_space() {
    int num_records;
    printf("Enter num of records: ");
    scanf("%d", &num_records);
    phonebook = (struct PHONE_RECORD*) malloc(sizeof(struct PHONE_RECORD)*num_records);
    if (phonebook == NULL){ 
        printf("Not enough memory.\n");
        return 1;
    }
    return num_records;
}

This resolves your segfault as far as the information you provided.

Use symbolic constants (NAME_LEN, BIRTHDAY_LEN, PHONE_LEN) instead of magic values (50, 12, 15).
Use local variables and pass whatever data they need to operate. This makes your code much easier to reason about.
Check the return value of scanf() otherwise you may be operating on uninitialized data.
Prefer using the variable instead of the type to sizeof(). It make type changes easier and less repetitive code.
Prefer using unsigned types when appropriate. What would num_records < 0 mean? Should 0 be a valid choice? malloc(0) is implementation defined so I disallowed it below.
It doesn't make sense to allocate space for your phonebook in the function called menu(). Moved it to main() instead.
Don't cast the void * from malloc.
(Not fixed) If you don't need the num_records value returned from num_space() then change the return type to void. If you do it, assign the return value to a variable.
(Not fixed) Consider using char * in struct phonebook instead of wasteful fixed sized strings. It typically means an allocation for each of the members but it's rather easy with strdup().
Minimized your code so you know what we expect of you:

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

#define NAME_LEN 50
#define BIRTHDAY_LEN 12
#define PHONE_LEN 15

struct phonebook {
    char name[NAME_LEN];
    char birthday[BIRTHDAY_LEN];
    char phone[PHONE_LEN];
};

size_t num_space(struct phonebook **phonebook) {
    size_t num_records;
    printf("Enter num of records: ");
    if(scanf("%zu", &num_records) != 1 || !num_records) {
        printf("scanf failed\n");
        return 0;
    }
    *phonebook = malloc(sizeof **phonebook * num_records);
    if (!*phonebook) {
        printf("malloc failed\n");
        return 0;
    }
    return num_records;
}

int main() {
    struct phonebook *phonebook = NULL;
    num_space(&phonebook);
    free(phonebook);
}

Using malloc to allocate space to a struct

1 Answers1