Why am I getting warnings when returning array from a function?

Question

I've seen a few examples of returning array from function on stackoverflow. I followed those examples but i am still getting warnings.

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

char * getNum();

int main(){

    int * num;
    int * i;

    num = getNum();
    puts(num);

    return 0;
}

char * getNum(){

    FILE * fp;
    fp = fopen("number", "r");      //getting a 1000 digit number from a file
    char * n;                       //putting it in "array"
    n = (char *)malloc(1000 * sizeof(char));
    char x[100];
    int i, j = 0;

    while(!feof(fp)){
        fgets(x, 100, fp);
        for(i=0; i<50; i++){        //getting the first 50 characters in a line
            n[j] = x[i];            //to avoid "new line"
            j++;
        }
    }
    fclose(fp);
    n[1000] = '\0';

    return n;
}

puts(num) gives the right number should I just ignore the warnings? Why are they popping up? I hope this isn't considered a duplicat.

cc     8.c   -o 8
8.c: In function ‘main’:
8.c:11:9: warning: assignment from incompatible pointer type
     num = getNum();
         ^
8.c:12:10: warning: passing argument 1 of ‘puts’ from incompatible pointer type
     puts(num);
          ^
In file included from 8.c:1:0:
/usr/include/stdio.h:695:12: note: expected ‘const char *’ but argument is of type ‘int *’
 extern int puts (const char *__s);
            ^

Answer 1

The problem is that you seem to be assigning the result of calling getNum to a variable of type int *, which doesn't make much sense as getNum returns a char *.

You also try to print with puts that variable of type int *, while puts accepts only a const char *.

What's more, you're getting out of bounds in your function's code, as I've already mentioned in the comments: n = (char *)malloc(1000 * sizeof(char)); allocates memory for exactly 1000 characters. n[1000] = '\0'; tries to access the 1001st (!) character. Remember, the array indices start from zero!

Answer 2

n = (char *)malloc(1000 * sizeof(char)); // ugly code

That should really be

 n = malloc(1000);
 if (n==NULL) {perror("malloc"); exit(EXIT_FAILURE);};

because sizeof(char) is always 1, and because you should not cast (in C) the result of malloc, and because malloc can fail and you should test against that.

Then you do later

   n[1000] = '\0'; // wrong code

This changes the 1001^-th byte (not the last 1000^-th one), so is undefined behavior (it is a buffer overflow). You should be really scared, bad things could happen (and if they don't, it is because you are unlucky). You probably want

 n[999] = '\0'; // the 1000th byte

BTW, I guess you are using GCC or Clang on some POSIX like system. You really should take the habit of compiling with all warnings and debug info:

cc   -Wall -g  8.c   -o 8

^{(If using make you probably want CFLAGS= -Wall -g in your Makefile)}

You might want to learn more about instrumentation options, e.g. you could also pass -fsanitize=address to gcc

Then you can use the gdb debugger and perhaps also valgrind. But be sure to improve your own code till you get no warnings.

Of course num = getNum(); is incorrect. You are assigning a pointer into an integer (so you have a type mismatch). That is often wrong (and in the rare cases -not here- you would really mean it, you should have some explicit cast to show to the reader of your code - perhaps you next year - what you really wanted to do).

You want to convert a string to a number. Read more the documentation of the C standard library. You want to use atoi (type man atoi in a terminal, or read atoi(3)) then code:

num = atoi(getNum());

You are also coding:

while(!feof(fp)) // bad code

This is wrong (and a bit tricky, see this). feof is only valid after some read operation (and not before, see feof(3)). since you want to remove the ending \n (but read fgets(3) & strchr(3)...) you probably want instead

 do {
   if (NULL==fgets(x, 100, fp)) break;
   char*eol = strchr(x, '\n');
   if (eol) *eol = 0;
 } while (!feof(fp));

at last your code is also incorrect because malloc is not initializing the returned memory zone. So your n might have a zero byte in some unexpected place.

Perhaps you should read about the (POSIX specific) strdup(3) or getline(3). Both could make your code simpler. And think about the adverse situation of files with very long lines (you might have a file with a line of many thousand bytes, and you could even have a file without any newlines in it).

Your puts(num) is also wrong, since also a type mismatch. Consider using printf(3) (but take the habit of ending the control format string with \n or else use fflush(3) because <stdio.h> is buffering), so printf("%d\n", num);

Answer 3

The problem is that you are assigning a pointer of type char * to one of type int *, hence "INCOMPATIBLE" pointers.

You also attempted to pass the int * pointer to puts() which expects a char * pointer, thus there is another warning.

Why are you doing this? If they are clearly pointers of different type it's very strange that you attempted it anyway.

It would be interesting to know, if you think this is correct for some reason.

You need to carefully read warnings and try to understand what they mean, it would help you learn a lot of the basics and good practice too.

Also, please note that: while (!feof(fp)) is not a good way to check if there is nothing else to read because fgets() needs to fail first (to attempt a read after EOF) so you still do the for (i ...) loop after the bad read. You should change to while (fget(x, 100, fp) != NULL)

Finally, in c there is no need to cast void * to any other poitner type, and it's considered bad practice to cast the return value of malloc() because of the reasons explained in this answer