How to properly allocate enough memory (malloc) when creating a new struct in C?

Question

Given the struct below, I am creating a function that takes in a person_in_queue and a position_num and allocating a new queue_t struct that is added to the end of a list of queue_t structs as specified by the first argument.

typedef struct queue {
  int position_num;
  char *person_in_queue;

  struct queue *next_in_line;
} queue_t;

I have written my code as such:

queue_t *add_to_queue(queue_t *input_queue, char *person_in_queue, int position_num) {

  input_queue = malloc(sizeof(queue_t));
  assert(input_queue != NULL);

  input_queue->position_num = position_num;
  input_queue->person_in_queue = (char *) malloc((strlen(new_text) + 1) * sizeof(char));
  assert(input_queue->person_in_queue != NULL);

  strcpy(input_queue->person_in_queue, person_in_queue);
  return input_queue;

}

Said code compiles, however, I am told that my code fails given that less memory is allocated than what is expected. At the moment, I'm not sure where I am going wrong here. Do note that I need to use malloc()!

Many thanks!

Well just taking a quick glance: `input_queue` is of type `queue_t *` yet you are doing `malloc(sizeof(operation_t))`. Apart from that: you're taking `input_queue` as argument and immediately overwriting it as the first thing in your function, then returning it. That doesn't make much sense and makes passing it as argument totally unneeded. — Marco Bonelli, Mar 24 '20 at 04:26
This question is tagged C. Don't cast malloc() in C. It can lead to an error. Casting malloc() is a C++ habit. https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc — hellork, Mar 24 '20 at 04:28
It is not required to use `* sizeof(char)` either. The C standard specifies char as one byte so `sizeof(char)` always returns 1. — hellork, Mar 24 '20 at 04:48
You are getting closer -- but ***note:*** when you edit your question, **do not** delete the original question. Instead **add** updates or edit at the end of your question. Why? Because removing parts of your original question will render all comments and answers that related to the deleted parts of your question -- meaningless. — David C. Rankin, Mar 24 '20 at 06:39
You cannot make the assertion that malloc did not return NULL, as malloc may return null. You need to check it, and using `assert` is not error checking. — William Pursell, Mar 24 '20 at 11:50
What is the point of passing an argument when the first thing your function does is overwrite the value that was passed? — William Pursell, Mar 24 '20 at 11:50
You ought not use the `_t` suffix for you own use: https://www.gnu.org/software/libc/manual/html_node/Reserved-Names.html — William Pursell, Mar 24 '20 at 11:54

hellork · Answer 1 · 2020-03-24T04:46:42.530

0

sizeof is an operator in C, not a funciton, but parenthesis are necessary to evaluate types.

To allocate memory for the struct, use the size of the type.

input_queue = malloc(sizeof (queue_t));

Or use the de-referenced pointer or object size (parenthesis not necessary here).

input_queue = malloc(sizeof *input_queue);

edited Mar 24 '20 at 04:46

answered Mar 24 '20 at 04:37

hellork

324
2
8

Parentheses are necessary when taking the size of the type ... and you can avoid a mismatch by specifying an object of the type instead , i.e. `*input_queue` – M.M Mar 24 '20 at 04:38
My memory has fizailed. (I'm used to using the object size [dererferenced pointer]. – hellork Mar 24 '20 at 04:41

score 0 · Answer 2 · answered Mar 24 '20 at 11:46

I am told that my code fails given that less memory is allocated than what is expected.

That must refer to the malloc((strlen(new_text) + 1) * sizeof(char)). Apparently new_text is a global string with no visible connection to person_in_queue, which latter is to be duplicated. Change the call to malloc(strlen(person_in_queue) + 1).

How to properly allocate enough memory (malloc) when creating a new struct in C?

Apart from the above, the allocation is basically okay, but as Marco Bonelli noted, you're taking input_queue as argument and immediately overwriting it … That doesn't make much sense … It would rather make sense to return the allocated queue_t object if input_queue is initially NULL, otherwise the passed input_queue unchanged. This can be accomplished by changing the first two statements of your function's body to

    queue_t *head_queue = input_queue, **pr = &input_queue;
    while (*pr) pr = &(*pr)->next_in_line;  // find end of list
    *pr =   // link new struct to list
    input_queue = malloc(sizeof(queue_t));
    assert(input_queue != NULL);
    input_queue->next_in_line = NULL;       // don't forget to initialize!

and the return statement to

    return head_queue ? head_queue : input_queue;

- the former also sets the link pointers next_in_line correctly.

How to properly allocate enough memory (malloc) when creating a new struct in C?

2 Answers2