-2

I would like to ask, what am I doing wrong with memory management. I just want to read some int array from stdin and then print it. Starting from 2 elements, then allocating by 2 to amount I will be satisfied with.

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

void read (int * array, int * n, int * l) {
    int * tmp;
    printf("Enter values:\n");
    for (*l = 0; *l < *n; (*l)++) {
        if (*l == *n - 1) {
            (*n) *= 2;
            tmp = (int*) realloc (array, sizeof(int) * (*n));
            if (tmp != NULL) {
                array = tmp;
                free(tmp);
            }
            else {
                printf("Error!\n");
                free(tmp);
        }
        if (scanf("%d", &array[*l]) != 1) break;
    }
}

void print (int * array, int length) {
    int i;
    printf("Your values:\n");
    for (i = 0; i < length; i++) printf("%d ", array[i]);
}

int main (void) {
    int n = 2; /* number of array elements */
    int length = 0; 
    int * array = (int *) malloc(sizeof(int) * n);

    read(array, &n, &length);
    print(array, length);
    free(array);
    return 0;
}
Dmytro O
  • 406
  • 5
  • 11
  • 9
    You never pass the value returned by `realloc` back to `main`, so the next line in `main` uses the old, freed pointer value – M.M Dec 08 '17 at 01:44
  • 6
    [Don't cast the result of `malloc` in C](http://stackoverflow.com/q/605845/995714) – phuclv Dec 08 '17 at 01:45
  • @Luru Vinh We sometimes do need to cast, in case you are allocation memory to struct block, how will malloc understand it? – Tushar Sharma Dec 08 '17 at 01:51
  • 5
    @TusharSharma: `malloc()` only understands the size of the memory, nothing else. – Mark Benningfield Dec 08 '17 at 02:00
  • `malloc()` returns a `void*`. Assigning that to a real pointer will automatically make it the right size, no cast needed. Of course, you might have to cast the real pointer you assign it to to something else later. – Lee Daniel Crocker Dec 08 '17 at 02:00
  • Sorry for asking silly questions , thanks for answer. I got the point my bad. – Tushar Sharma Dec 08 '17 at 02:09
  • So should I pass this array to function as read(&array) ..., if I want to modify it in outside the main ? – Dmytro O Dec 08 '17 at 02:10
  • To reassign the array in your `read` function, you need to pass an `int**`; but that doesn't do you any good either, because you free the memory you just reallocated with this `free(tmp)`. – Mark Benningfield Dec 08 '17 at 02:11
  • Possible duplicate of [Initializing a pointer in a separate function in C](https://stackoverflow.com/questions/2486235/initializing-a-pointer-in-a-separate-function-in-c) – Bo Persson Dec 08 '17 at 04:46

2 Answers2

1

regarding: free(tmp);

This statement should not be anywhere in the posted code.

It frees the memory just allocated

==== the pointer 'n' better be pointing to an array, in the caller, that was allocated and not to some fixed array

==== When passing a pointer where the called function is going to change where that pointer points, It must be passed (in the current scenario) as int **n. This also means the calling function must pass the address of the pointer, not the contents of the pointer

==== variable (and parameter) names should indicate usage or content (or better, both) the parameter names 'n' and 'l' are meaningless even in the current context

user3629249
  • 16,402
  • 1
  • 16
  • 17
  • Ok, I removed all free(tmp) from read() and it started work. That's fine, but why don't I need to free this new memory? Is it freeing after automatically after I left function read()? – Dmytro O Dec 08 '17 at 17:27
  • @DmytroOsaulenko, Suggest reading/understanding the MAN page for `realloc()` as that should answer your question. – user3629249 Dec 09 '17 at 08:27
0

Remove the free after a successful realloc. Realloc takes care of that for you. Also, if you ever hit the error case, you will double free() your buffer.

mevets
  • 10,070
  • 1
  • 21
  • 33