Append function for C with dynamic memory allocation

Question

Following this question, I'm trying to modify the code provided in this blog post to create an append function using dynamic memory allocation. This is what I have so far:

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

typedef struct intlist_ {
  int size;
  int* list;
} intlist;

void append(intlist* arr, int value){
  realloc((*arr).list, sizeof((*arr).list) + sizeof(int));
  (*arr).size = (*arr).size + 1;
  (*arr).list[(*arr).size -1] = value;
}

int main() {

  intlist arr;
  arr.size = 4;
  arr.list = malloc(arr.size * sizeof(int));

  arr.list[0] = 0;
  arr.list[1] = 5;
  arr.list[2] = 3;
  arr.list[3] = 64;

  append(&arr, 12);

  for (int ii = 0; ii < arr.size; ii++)
    printf("%d, ", arr.list[ii]);

  free(arr.list);
  return 0;
}

However the result I get is wrong:

clang version 7.0.0-3~ubuntu0.18.04.1 (tags/RELEASE_700/final) main.c:10:3: warning: ignoring return value of function declared with 'warn_unused_result' attribute [-Wunused-result] realloc((*arr).list, sizeof((*arr).list) + sizeof(int)); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1 warning generated. 0, 5, 3, 64, 0, 5, 3, 64, 12,

I'm using this online compiler for testing where you can also see the latest versions of the above code. I would appreciate if you could help me know where is my mistake and how I can solve it. Thanks for your support in advance.

P.S. You may a final version of the code here in this Gist.

Why do you think the result is wrong? I tried to compile it and I get correct result. — Emir, Apr 14 '19 at 21:06
@EmirG. you are right. I have made horrible mistake in my original post forgetting to comment the first for loop out. :( shame. However The warning I'm getting is still an issue. — Foad S. Farimani, Apr 14 '19 at 21:12

Heath Raftery · Answer 1 · 2019-04-15T06:35:07.250

2

Close, but:

sizeof((*arr).list) wont give you the size of the array. Instead it will give you the size of a int*.
realloc invalidates the original pointer, and continuing to use it is undefined behaviour. Use the returned value instead.

So change your realloc line to use the stored list size instead, and update the pointer with the return value:

(*arr).list = realloc((*arr).list, ((*arr).size + 1) * sizeof(int));

Couple of other tips:

ptr-> is the same as (*ptr). but easier to read. I suggest changing all your (*arr).size and (*arr).list to arr->size and arr->list.
realloc, like its brethren, is not guaranteed to succeed. You should check the return value for null to catch errors.
The clang warning is (as often is the case) helpful - checking the return value would have solved a couple of issues.

edited Apr 15 '19 at 06:35

answered Apr 14 '19 at 21:09

Heath Raftery

3,643
17
34

thanks. it seems the [original post](https://medium.com/@imjacobclark/working-with-dynamic-arrays-in-c-c7d40a3cea01) is also wrong then. However your solution did not remove the warning. – Foad S. Farimani Apr 14 '19 at 21:15
1

`realloc((*arr).list, ((*arr).size + 1) * sizeof(int));` --> Nope. the reallocated value is discarded. Only by UB are things still working. – chux - Reinstate Monica Apr 14 '19 at 21:22
@chux would you be kind to elaborate? It seems it is working fine. – Foad S. Farimani Apr 14 '19 at 21:27
2

@Foad Many reallocations do not use a new memory area, your code so far relies on that. Keep calling your/this `append()` and code will fail. – chux - Reinstate Monica Apr 14 '19 at 21:36

chux - Reinstate Monica · Accepted Answer · 2019-04-14T21:52:28.543

1

Wrong size allocated. sizeof((*arr).list) is the size of a pointer, not int.
Return value not used. realloc() returns the new pointer.
No NULL check

Rather than use the error prone ptr = some_alloc(sizeof(type) * n), use ptr = some_alloc(sizeof *ptr * n)

void append(intlist *arr, int value){
  int *new_ptr = realloc(arr->list, sizeof *(arr->list) * (arr->size + 1u));
  if (new_ptr == NULL) {
    fprintf(stderr, "Out of memory\n");
    exit (EXIT_FAILURE);
  }
  arr->list = new_ptr;
  arr->list[arr->size] = value;
  arr->size++;
}

edited Apr 14 '19 at 21:52

answered Apr 14 '19 at 21:32

chux - Reinstate Monica

143,097
13
135
256

thanks a lot. I do not understand the `*(arr->list)` and `1u`. I also think the line `arr->list[arr->size] = value;` should be `arr->list[arr->size - 1] = value;` shouldn't it? – Foad S. Farimani Apr 14 '19 at 21:39
2

@Foad `sizeof *(arr->list)` is the size of what `arr->list` points to. With `int* list;`, the type pointer to in a `int`. – chux - Reinstate Monica Apr 14 '19 at 21:42
1

BTW the `new_arr == NULL` should change to `new_ptr == NULL` – Foad S. Farimani Apr 14 '19 at 21:43
1

@Foad Allocations take a `size_t` argument, some unsigned type. `arr->size + 1u` adds 1, using `unsigned` math rather than `int math` in forming the size product. – chux - Reinstate Monica Apr 14 '19 at 21:44
2

@Foad "I also think the line arr->list[arr->size] = value; should be arr->list[arr->size - 1] = value; shouldn't it?". No. In effect you want `arr.list[4] = 12;` This code does the assignment before incrementing `.size`. – chux - Reinstate Monica Apr 14 '19 at 21:47
Thanks for your kind support. you helped me a lot. – Foad S. Farimani Apr 14 '19 at 21:50

Append function for C with dynamic memory allocation

2 Answers2