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
– Emir, Commented Apr 14, 2019 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
– Foad S. Farimani, Commented Apr 14, 2019 at 21:12

Heath Raftery · Accepted Answer · 2019-04-15 06:35:07Z

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, 2019 at 6:35

answered Apr 14, 2019 at 21:09

Heath Raftery

4,35728 silver badges47 bronze badges

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

4 Comments

Foad S. Farimani Over a year ago

thanks. it seems the original post is also wrong then. However your solution did not remove the warning. 🤔

chux Over a year ago

realloc((*arr).list, ((*arr).size + 1) * sizeof(int)); --> Nope. the reallocated value is discarded. Only by UB are things still working.

Foad S. Farimani Over a year ago

@chux would you be kind to elaborate? It seems it is working fine.

chux Over a year ago

@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 · Accepted Answer · 2019-04-14 21:52:28Z

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, 2019 at 21:52

answered Apr 14, 2019 at 21:32

chux

158k17 gold badges160 silver badges311 bronze badges

6 Comments

Foad S. Farimani Over a year ago

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?

chux Over a year ago

@Foad sizeof *(arr->list) is the size of what arr->list points to. With int* list;, the type pointer to in a int.

Foad S. Farimani Over a year ago

BTW the new_arr == NULL should change to new_ptr == NULL

chux Over a year ago

@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 Over a year ago

@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.

|

Collectives™ on Stack Overflow

Append function for C with dynamic memory allocation

2 Answers 2

4 Comments

6 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

4 Comments

6 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related