1

I have the following code that is supposed to create a new array with the number of elements specified in the parameter...

void *newArray(int nElements){
    int *array = NULL;
    array = (int *)malloc(nElements * sizeof(int));
    if(array == NULL){
        printf("Insufficient memory space.\n");
        return -1;
    }
    //Set all values to 0
    for(int i = 0; i < nElements; i++){
        array[i] = 0;
    }
    return array;
}

This function returns a generic pointer to an array of type integer with all its values started at 0. But when I invoke the call to this function...

int *array = NULL;
array = (int *)newArray(5);

Throws me the following warning:

main.c:24:16: warning: incompatible integer to pointer conversion returning 'int' from a function with result type 'void *' [-Wint-conversion]

and if I try to print the contents of the array print me two 0. I'm still getting familiar with the C language and I don't really understand what that warning means and how I can fix it so that the function returns me an array with 5 elements in that case.

Barmar
  • 741,623
  • 53
  • 500
  • 612
  • 3
    I removed the C++ tag from your question, since you stated that this is about C. C and C++ are different languages, and pointer conversions is one area where they can vary significantly. – Brian61354270 Aug 22 '21 at 15:51
  • BTW, you can use `calloc()` to allocate the memory and set it to `0` automatically. – Barmar Aug 22 '21 at 16:01
  • Change return type of `newArray()` to `int *`. I think it is obvious. – i486 Aug 22 '21 at 16:27
  • @i486 there is no difference - only you will not have warning when assigning to other pointer types – 0___________ Aug 22 '21 at 16:31
  • @0___________ or maybe the error is from line `return array;` With `int *` funtion type there will be no error. Other solution is `return (void *)array;`. But I don't understand why `void *` type when it is always used for `int *`. – i486 Aug 23 '21 at 06:01

2 Answers2

3

Your function has the return type void*. Which will make C try to convert the integer literal -1 in return -1 to a void*, which an "incompatible conversion". This conversion is possible, but it is generally not what you want (as opposed to an float to double conversion, for example).

To solve this, simply return the null pointer constant NULL instead.

Some additional advice:

  • Look at the parameter type for malloc and the sizeof operator, it is size_t. You may want to have your function accept a size_t as well instead. The "size type" is guaranteed to be large enough for any object you could theoretically store, and cannot be negative.
  • Setting an entire array of int types to zero can be performed by calling memset. The action of first allocating memory and then setting it all to zeros can be combined using one function call to calloc.
  • malloc and calloc return NULL on failure. Therefore you can write:
int *array = (int*) malloc(nElements * sizeof(int));
    if(array == NULL){
  • The return type could also be int*, as that type is hardcoded into the function. But perhaps you were gradually writing a calloc-like wrapper, which prints an error message in case the allocation failed?
Yun
  • 3,056
  • 6
  • 9
  • 28
  • 1
    Indeed the problem was in that return -1. I apreciate it – Jairo Flores Aug 22 '21 at 16:16
  • do not cast result of malloc – 0___________ Aug 22 '21 at 16:28
  • memset will only work with `0`. Any other values will have to be manually set in the loop – 0___________ Aug 22 '21 at 16:30
  • @0___________ I am aware of [this](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) discussion and consider it disputed. It's not strictly necessary and does add some clutter, but on the other hand, there's also something to be said for having no implicit conversions. – Yun Aug 22 '21 at 16:33
  • Another discussion is `sizeof(..)` vs. `sizeof *`, but I'm not going to write an entire C-course here :) – Yun Aug 22 '21 at 16:34
  • @that discussion is prehistoric :) Novadays too many people try to compile the C code using C++ compilers. That is the reason to not cast. If it will not compile - tell your compiler to comple as C – 0___________ Aug 22 '21 at 18:02
  • Hah :) I see C and C++ as separate languages. Do you have a reference that shows that this is relevant problem and the right solution? If there is, I will be convinced to not cast. – Yun Aug 22 '21 at 18:05
1
    return -1;

You return -1 which is integer and your function returns pointer.

The most common way of signaling the caller that something has gone wrong is to return NULL.

void *newArray(size_t nElements){
    int *array = NULL;
    array = malloc(nElements * sizeof(*array));
    if(array == NULL){
        printf("Insufficient memory space.\n");
    }
    else
    {
       //Set all values to 0
       for(size_t i = 0; i < nElements; i++){
           array[i] = 0;
    }
    return array;
}

As you see you do not have to add a second return. Generally, for code clarity, it is good to avoid many returns in the function.

Some additional remarks:

  1. Do not cast the malloc return pointers. If your code does not compile it maens that you compile C code using C++ compiler and it is wrong. Most C++ compilers have command line options to set the language (for example g++ -xc)
  2. Use objects not types in sizeof
  3. Use size_t for sizes & array indexes.

I do not know why you return void * instead of int *, but maybe you have some reasons.

0___________
  • 60,014
  • 4
  • 34
  • 74