1

What have I done wrong here? It falls after I input count and then my array It must be something with memory allocation, but i tried many things and none of them helped. thanks for responding

struct _arr {
   size_t count;
   int* arr;
};

typedef struct _arr array_t;

array_t array_create(int* arr, size_t count){
    array_t* newArr;

    newArr = (array_t*)malloc(sizeof(count)+sizeof(int)*count);
    newArr->count = count;
    newArr->arr = arr;
    return *newArr;
}


array_t array_get(FILE* file){
    int* arr = NULL;
        size_t count;
    array_t arr_t;
    int i = 0;

    if (!file) return;
    if (!fscanf(file, "%u", &count)) return;
    arr_t = array_create(arr, count);

    for (i = 0; i < arr_t.count; i++){
        if (!fscanf(file, "%d", &arr_t.arr[i])) return;
    }

    for (i = 0; i<arr_t.count; i++)
    printf("%d ", arr_t.arr[i]);
    printf("\n");

    return arr_t;
}


int main(){
    array_t arr;
    int i = 0;

    arr = array_get(stdin);

    for (i = 0; i<arr.count; i++)
        printf("%d ", arr.arr[i]);

    getch();
    return 0;
}
  • 3
    If you think `array_create()`, conveniently omitted from the posted source, is somehow insignificant and has nothing to do with your problem, you're probably mistaken. Post it. – WhozCraig Nov 16 '13 at 19:23
  • 3
    Your function sometimes returns nothing, does your compiler not tell you about that? – Charlie Burns Nov 16 '13 at 19:27
  • @CharlieBurns outstanding catch. the indentation and single-line if-logic totally hid that from my tired eyes. – WhozCraig Nov 16 '13 at 19:28
  • what is arr in array_create()? –  Nov 16 '13 at 19:28
  • 1
    Do you have a compiler?? – user1338 Nov 16 '13 at 19:29
  • 1
    In case you don't understand what Charlie said, let me spell it out for you. You're function is defined to return an `array_t`. Anywhere in that function that is simply `return;` is undefined behavior. – WhozCraig Nov 16 '13 at 19:32
  • @WhozCraig, yeah it's an issue, but not his current problem if he is seeing the printf's in the function working. I'd like to see array_create. – Charlie Burns Nov 16 '13 at 19:33
  • @CharlieBurns you and I both at this point. Still a solid catch on your part, and definitely a problem none-the-less. – WhozCraig Nov 16 '13 at 19:34
  • My guess is that arr.arr is not properly allocated. – Charlie Burns Nov 16 '13 at 19:34
  • i accidently deleted `arr` declaration. temporary i made it `int arr[3]` i'm stupid. but now i returned to my last problem. i'll edit my post now –  Nov 16 '13 at 19:36
  • Please note that the condition `if (!fscanf(file, "%d", &arr_t.arr[i])) return;` is wrong : fscanf returns -1 on EOF, which also evaluates as true. (which would cause the program to loop and put garbage values into arr[].) Hey, wait ... – wildplasser Nov 16 '13 at 19:42
  • @wildplasser Minor: `fscanf()` returns `EOF` on end-of-file, not necessarily `-1`. – chux - Reinstate Monica Nov 16 '13 at 19:53
  • 1
    There is no need to dyna-allocate the entire structure. C supports by-value copying of structures. and the only thing that is variant in your case is the length of the allocated vector. The allocator in Charlie's answer is a significantly better approach. Also, your math is wrong if you're going to do it as you are (which you shouldn't be, and leaks memory, btw). The math should be `sizeof(*newArr) + count*sizeof(int)` *but that is not the way to fix this*. – WhozCraig Nov 16 '13 at 19:56

2 Answers2

2

WhozCraig's crystal ball says your problem is in array_create(). Specifically, the field arr is not allocated properly. Your array_create() should look something like this:

array_t array_create(int length)
{
    array_t res = {length, NULL};
    if (length > 0) {
        res.arr = malloc(length * sizeof(*res.arr));
        if(res.arr == NULL) {
            printf("malloc(%d) failed\n", length * sizeof(*res.arr));
            exit(1);
        }
    }
    return res;
}

You also need to fix those empty returns. Maybe you just want to print a message and exit.


Update Mon Nov 18 16:13:48 CST 2013

This is a whole different version that allocates an array_t from the heap. The error handling is kind of ugly ( all those places that return 0; ). This should work, the other worked too, but maybe an array_t on the heap is what you are looking for.

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

typedef struct {
   size_t count;
   int *arr;
} array_t;

array_t *array_create(size_t count){
    array_t *newArr = malloc(sizeof(array_t));
    if(newArr == 0) return 0;
    newArr->count = count;
    newArr->arr = calloc(sizeof(int),count);
    if(newArr->arr == 0) return 0;
    return newArr;
}

array_t *array_get(FILE* file){
    size_t count;
    array_t *arr;
    int i = 0;

    if (!file) return 0;
    if (!fscanf(file, "%zu", &count)) return 0;
    arr = array_create(count);
    if(arr == 0) return 0;

    for (i = 0; i < arr->count; i++){
        if (!fscanf(file, "%d", &arr->arr[i])) return 0;
    }

    for (i = 0; i<arr->count; i++)
        printf("%d ", arr->arr[i]);
    printf("\n");

    return arr;
}


int main(void) {
    array_t *arr;
    int i = 0;

    arr = array_get(stdin);
    if(arr == 0) return -1;

    for (i = 0; i<arr->count; i++)
        printf("%d ", arr->arr[i]);

    getch(); // getch is windows
    return 0;
}
Charlie Burns
  • 6,994
  • 20
  • 29
  • We *have* to post a version that works. We can't just say *that* (though I chuckled when I saw it pop up, to be sure). – WhozCraig Nov 16 '13 at 19:37
  • I'll delete if you want... I had 'balls' plural my mistake. That sounded kinda iffy. – Charlie Burns Nov 16 '13 at 19:39
  • 1
    Like in: `my balls feel like a pair of maraka's` ? – wildplasser Nov 16 '13 at 19:44
  • i added my `array_create` function. and i have to pass both `arr` and `count` as arguments. it's a requirement –  Nov 16 '13 at 19:56
  • What you are doing doesn't make sense. If arr has to be passed in, what is it supposed to contain or be set to upon return. – Charlie Burns Nov 16 '13 at 20:00
  • @user3000073 A requirement *of what*? And what *exactly* is the purpose of that parameter? I can *almost* see passing `arr` *by address* as a requirement, but then the function return value makes no sense to have. – WhozCraig Nov 16 '13 at 20:01
  • You are close to a common C idiom but I can't believe they are trying to teach that to you yet, so I'm not going to mention it. – Charlie Burns Nov 16 '13 at 20:03
  • well, i know that i do something wrong here. `array_create` must allocate space on the heap and correctly initialize `array_t` variable by passed `count` and `arr` –  Nov 16 '13 at 20:04
  • We don't have your assignment with it's requirements in front of us. We have a given you what we think is the best answer. If you are required to pass in arr for some reason, we don't really know why. You could use the code above, pass in arr and just ignore it. That would make your code work. – Charlie Burns Nov 16 '13 at 20:07
  • but how can i pass a valid `arr` when i just want to allocate memory for it and don't know whar `arr` contains yet –  Nov 16 '13 at 20:08
  • @charlie-burns can you explain your solution please? you are returning a local variable and not a pointer to the place on the heap –  Nov 18 '13 at 21:31
  • Since your original code was passing around structs and not pointers to structs in the heap, we assumed this is what you wanted so we provided answers in that domain. If array_create() was supposed to allocate and return a pointer to a struct in the heap, then a different answer is needed. I just posted a whole different version below the original answer above. Hope this helps. – Charlie Burns Nov 18 '13 at 22:17
0

according to me you are trying to returning the value that present on that address having pointer. so just simply return newArr

asifaftab87
  • 1,315
  • 24
  • 28