0

I created a function that fills the values of a struct with ints and is supposed to return a pointer to that same struct. However, when I print what the pointer references, it prints garbage values.

Here is my code: 

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

struct Map * collect_values(int n, int *arr);

struct Map{
    int value, position;
};

int main(){
    int size, i;
    scanf("%d", &size);
    int *arr = (int*) malloc(size*sizeof(int));
    struct Map *p = collect_values(size,arr);

    for(i = 0; i < size; i++){
        printf("%d : %d\n", p[i].value, p[i].position);
    }
    return 0;
}

struct Map * collect_values(int n, int *arr){
    int i, position = 0;
    struct Map array[n];
    for(i = 0; i < n; i++){
        scanf("%d",&arr[i]);
        array[i].value = arr[i];
        array[i].position = position;
        position++;
    }
    struct Map *ptr = &array[n];
    return ptr;
}

I'm pipelining values from a file, so the scanf() works fine, and I've printed from the collect values() so I know that it is creating the struct properly.

However, when I print from the main method, the output is: 

-485221568 : 32766
-1529319768 : 32767
24 : 48
-485221520 : 32766
-485221776 : 32766
32766 : 0
872480919 : -968757580
0 : 0
0 : 0
0 : 0

I've tried using p[i]->value, but it doesn't compile. What am I doing wrong?

mch
  • 9,424
  • 2
  • 28
  • 42
quazi_moto
  • 449
  • 1
  • 3
  • 14
  • 1
    I'll try to look for a duplicate, but you're returning a pointer to a local variable. – melpomene Sep 13 '18 at 05:39
  • In second last line, try `struct Map *ptr = &array;` or `struct Map *ptr = &array[0];` –  Sep 13 '18 at 05:45
  • @psinaught The first one is a type error, the second one doesn't fix the problem (returning a pointer to a local variable). – melpomene Sep 13 '18 at 05:51
  • @melpomene did you find a duplicate? I'm still not really sure how to fix it. I'm not sure which struct is the local variable that is out of scope. Is it array[n]? Or ptr? They are both created locally in the collect values function. – quazi_moto Sep 13 '18 at 06:00
  • By the way, you're returning the address of an element that doesn't exist. `array` has `n` elements, so the valid indices range from `0` to `n-1`. `&array[n]` is the address of an element one past the end of the array. Both `array` and `ptr` are local variables and die at when the function returns, but `return ptr` means a *copy* of `ptr` is returned, so this part is fine. However, this pointer points to an element of `array`, which no longer exists and that's the problem. I have found duplicates; scroll to the top (and maybe reload the page) if they don't show up for you. – melpomene Sep 13 '18 at 06:03

1 Answers1

0

The struct is declared locally, and therefore "dies" after its scope (i.e. the function it was declared in) has ended, and that causes the "garbage" values.

You should either

  1. create the struct locally in the caller function and pass it (by address) to the filling function
  2. create the struct using malloc in the filling function, return its pointer, and free it in the caller function after you are done with it
CIsForCookies
  • 12,097
  • 11
  • 59
  • 124
  • Do you mind being a little more explicit? For your second option, which is what I think I want to do because I want to keep my main method lean (I'm actually going to send that pointer to a different function, but I wanted to see if it would print in the first place), I should do something like this in the collect_values(): struct Map *ptr; ptr = malloc(sizeof(struct Map)); return ptr; And then free in the main method? But how will it know to point to the array[n] struct if I never say to do so? – quazi_moto Sep 13 '18 at 05:51
  • @Rahme You shouldn't have an `array[n]` (which by the way is an array, not a struct). `array` being a local array is what the whole problem is about. Instead you should do `struct Map *array = calloc(n, sizeof *array); ... return array;` and then in `main` `free(p);` when you're done using it. – melpomene Sep 13 '18 at 05:59
  • Suppose I pass *p from the main to a different function to sort the struct. Do I free the memory in that function as well? Or just in the main? When do I need to free the memory? – quazi_moto Sep 13 '18 at 06:09
  • free should occur only once, when you are done with the memory. A good practice is to do that from the function that allocated the memory (but if you do it from its caller / callee it would also work, albeit harder to understand) – CIsForCookies Sep 13 '18 at 06:24