struct with string including proper memory management

Question

I'm trying to implement a struct which contains strings (char *). I'm a bit lost as I'm able to compile my code but it's crashing without additional information from the OS (most likely a memory issue).

My struct:

typedef struct student 
{
   int matrikelnummer;
   char *name;
   struct student *next;
} student;

While trying to properly allocate the memory for *name I'm failing:

void insert_student(student **hash_table)
{
    char *name;
    int matrikelnummer;

    student *temp = malloc(sizeof(student));
    if (temp == NULL)
        return;

    printf("Neuer Student hinzufuegen:\n");
    printf("Name: ");
    scanf("%s", &name);

    temp->name = malloc(strlen(*name) + 1);
    if (temp->name == NULL)
        return;
    strcpy(temp->name, *name);

    printf("Matrikelnumer: ");
    scanf("%i", temp->matrikelnummer);

    temp->next = NULL;

    hash_table[get_hash_key(temp->matrikelnummer)] = &temp;
}

The program is crashing right after:

scanf("%s", &name);

I already read through some strcpy/strdup discussions but I thought this should work as implemented.

I'm sure it's a memory issue but I tried more or less all different types of declaring/allocating I'm aware of (doesn't mean too much ;)).

Any help appreciated.

*I know that there is some stuff missing when it comes to proper use of hash tables/linked list. It's just the beginning of a new little project for study purpose.

edit - adding FIXED function - void insert_student(student **hash_table):

void insert_student(student **hash_table)
{
    char *name = malloc(100);
    student *temp = malloc(sizeof(student));    
    if (temp == NULL || name == NULL)
        return;

    printf("Neuer Student hinzufuegen:\n");
    printf("Name: ");
    scanf("%99s", name);
    temp->name = malloc(strlen(name) + 1);
    if (temp->name == NULL)
        return;
    strcpy(temp->name, name);        
    free(name);
    name = NULL;

    printf("Matrikelnumer: ");
    scanf("%d", &temp->matrikelnummer);

    temp->next = NULL;

    hash_table[get_hash_key(temp->matrikelnummer)] = &temp;     
}

1. Read the manual page for scanf. 2. Allocate some memory for name — Ed Heal
– Ed Heal, Commented Dec 25, 2016 at 12:59

gsamaras · Accepted Answer · 2016-12-25 14:18:48Z

4

First of all, you don't allocate memory for name, just as you did for temp->name, it's the same logic:

char *name;
// no alloacation for 'name' in code here
scanf("%s", &name); // bomb ready to explode here

Also, since name is of type char*, you want:

scanf("%s", name);

since from the ref, we have that the prototype of scanf() is:

int scanf ( const char * format, ... );

As a result, your function could look like this now:

void insert_student(student **hash_table)
{
    char *name;
    ...
    name = malloc(100); // arbitrary max length of name (99 + null terminator)
    if (name == NULL)
        return;
    printf("Name: ");
    scanf("%s", name);

    // rest of the code seems fine at a glance
}

of, if I were you, since you don't really need dynamic allocation for name, since you have to set an arbitrary max length for your string, you could do:

#define MAX_LENGTH 100

void insert_student(student **hash_table)
{
    char name[MAX_LENGTH];
    ...

    printf("Name: ");
    scanf("%s", name);

    // rest of the code seems fine at a glance
}

PS - As Ed Heal suggested, this would be better:

scanf("%99s", name); // a max length of 99 for the name

as I discuss in fgets() vs scanf() – overflow.

edited Dec 25, 2016 at 14:18

answered Dec 25, 2016 at 13:00

gsamaras

73.7k50 gold badges210 silver badges330 bronze badges

Sign up to request clarification or add additional context in comments.

6 Comments

Ed Heal Over a year ago

And you do not need the &

gsamaras Over a year ago

Sure @EdHeal, happy that we both see it at the same way. Hope you like my answer now. PS - Merry Xmas =)

Ed Heal Over a year ago

I have +1 - Happy xmas as well (even though that I am an atheist so not celebrating it in any way)

Ed Heal Over a year ago

Perhaps scanf("%99s".name);` would be better

Mike Bessonov Over a year ago

You should also provide the maximum allowed string size in the scanf() format string, like scanf("%20s", name). The name var can be declared on the stack as char name[21]; (with extra char for the trailing zero). And using -Wall option of the gcc compiler can give you lots of useful warnings.

|

Sumit Gemini · Accepted Answer · 2016-12-25 13:03:16Z

0

your problem is you didn't allocate memory for name (char*name) and without allocating name with valid memory area you are going to initialize it.

 in insert_student(),

    char *name;     
    name=malloc("give some size to it"); 
    scanf("%s",name) // don't use & to get string because name is already treat as address.

answered Dec 25, 2016 at 13:03

Sumit Gemini

1,8461 gold badge15 silver badges20 bronze badges

Collectives™ on Stack Overflow

struct with string including proper memory management

2 Answers 2

6 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

6 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related