-1

I want to create a new intarr_t with initial size len, but I've never handled this type of problem with a typedef'ed variable.

My problem is that intarr_create() should allocate the array space and then return a pointer to it if malloc was successful or a pointer to NULL if I failed. How can I fix this?

Also, why there is a * symbol in the function?

Here's my code:

#include <stdio.h>

typedef struct {
    int* data;
    unsigned int len;
} intarr_t;

intarr_t* intarr_create(unsigned int len) { 
    //intarr_t with initial size len
    intarr_t = (int *) malloc(len); // not working here, can someone explain why?
    if(intarr_t != NULL) {
        return intarr_t;
    } else {
        return NULL;
    }
}

int main() {
    int len = 15;
    int h = intarr_create(len);
    printf("%d\n", h);
    return 0;
}   
BeginnerC
  • 119
  • 2
  • 14
  • Note that [`typedef`'ing structs](http://stackoverflow.com/a/4566358/1757964) is one of the worst and most horrid abuses of the C language. If you can avoid using it, please do so. – APerson Nov 05 '14 at 19:27
  • 1
    You are trying to assign a pointer type to a non-pointer type in your function call in `main`. You may as well just return the result of `malloc` if you are going to return NULL on fail anyway, [and don't cast the result of `malloc`](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) – IllusiveBrian Nov 05 '14 at 19:28
  • @APerson that's a bit extreme, it has some advantages and few drawbacks – M.M Nov 05 '14 at 19:51
  • @MattMcNabb Fair enough; but it causes enough maintainability headaches to be inadvisable in the long run. – APerson Nov 06 '14 at 00:31
  • @APerson I disagree based on experience. Namespace pollution problems can be avoided with good naming conventions. Naming a variable the same thing as its tag name is silly. YMMV. – M.M Nov 06 '14 at 01:51

3 Answers3

2

It's not working because you did not give your variable a name. Also, int* and intarr_t are not the same type, so you will get a type mismatch unless you change the cast.

Kevin
  • 28,963
  • 9
  • 62
  • 81
  • What do you mean by changing the cast? – BeginnerC Nov 05 '14 at 19:30
  • @BeginnerC you wrote `(int*) malloc(len)`. The `(int*)` is called a cast; it changes the type of the `malloc()` call from `void*` (a pointer to anything) to `int*` (a pointer to an integer variable). But you don't have an integer variable, so you need to change it to something like `(intarr_t*)`. You'll also need to make that variable into a pointer. You'll end up with something like `intarr_t *result = (intarr_t*) malloc(len);` – Kevin Nov 05 '14 at 19:33
  • Thank you for quick response, but I am slightly confused and forgive my slow understanding but since I want to return a pointer to the newly-allocated intarr_t If successful (i.e. memory allocation succeeds, would I be returning result? – BeginnerC Nov 05 '14 at 19:40
  • Most likely. You may not want to pass `len` to `malloc()` as-is, though. The argument to `malloc()` should be a number of bytes. You probably want to use `sizeof(intarr_t)` instead of a custom length. I imagine you'll also want to allocate an array of integers and return it in `result->data`, which requires a separate `malloc()` call (something like `result->data = (int*) malloc(len*sizeof(int))`). – Kevin Nov 05 '14 at 19:44
  • What kind of statement in my main should I be using if I want to store the return value? My main declaration of h being equal to it doens't seem to work. – BeginnerC Nov 05 '14 at 19:50
  • `h` needs to be a pointer to `intarr_t`. Right now, you've declared it as `int h`. You need to change it to something like `intarr_t *h`. – Kevin Nov 05 '14 at 19:53
1

Rewrite your function into this:

intarr_t* intarr_create(unsigned int len)
{ 
    intarr_t *result;

    result = (intarr_t *)malloc(sizeof(intarr_t));   // allocate memory for struct
    if(result != NULL)
    {
        result->data = (int *)malloc(len * sizeof(int));   // allocate memory for data
        result->len = len;
        if (result->data == NULL)
        {
           /* handle error */
        }
    }
    else
    {
        /* handle error */
    }

    return (result);
}

You have to do a "double" malloc to get it right. First you have to allocate the memory for the intarr_t and if that was successful you have to allocate the memory for the data array.

Additionally malloc returns a void * which must be cast to the correct pointer type (should be a warning or maybe even an error with some compilers).

Lukas Thomsen
  • 3,089
  • 2
  • 17
  • 23
  • This returns an array of intarr_t structs with no memory allocated for the data. I don't think this is what the OP wants. – Daniel Nov 05 '14 at 19:33
  • @Daniel According to the question this is exactly what the function should do (`My problem is that intarr_create() should allocate the array space and then return a pointer to it if malloc was successful or a pointer to NULL if I failed.`)... ...huups sorry missed a bit! – Lukas Thomsen Nov 05 '14 at 19:40
0

You have a few problems with your intarr_create function. First of all, you need to name your intarr_t variable. Now you have the slightly trickier problem of allocating memory for the actual array of integers in addition to your intarr structure. Remember, that you will have to call delete twice to destroy this object. Once on the data, and once on the actual structure itself.

intarr_t* intarr_create(unsigned int len)
{
    intarr_t* array = (intarr_t*)malloc(sizeof(intarr_t));
    array->data = (int*)malloc(len * sizeof(int));

    return array;
}
Daniel
  • 6,595
  • 9
  • 38
  • 70