tags:

views:

88

answers:

4

Why does this code produce the warning?

FILE* my_open_file(char* filename)
{
  FILE* fp=fopen(filename,"r");
  if(fp==NULL)
    {
      perror("Error opening file");
      return -1;
    }
  return fp;
}
  • asdf.c: In function ‘my_open_file’:
  • asdf.c:9: warning: return makes pointer from integer without a cast

fp is already a pointer, and not integer as far as I can see.

+7  A: 

The compiler doesn't like return -1, which is of type int—but my_open_file is supposed to return pointer-to-FILE.

Use return NULL; to signal error.

Greg Bacon
Or just a single `return fp;` since `fp` is known to be NULL.
Jonathan Leffler
+2  A: 

I think that this warning point to return -1

Instead of it just use "return NULL"

matekm
A: 

You're returning -1 on error, which then must be implicitly cast to a FILE*

James
+1  A: 

As others have said, you want to return a pointer in any case, because your function is declared to return a FILE *. So, you should return NULL if fopen() fails, and the return value of fopen() if it doesn't.

But then, your function is (almost) equivalent to fopen()! In other words, you can replace your function by:

FILE* my_open_file(char* filename)
{
    FILE *fp = fopen(filename);
    if (fp == NULL) {
        fprintf(stderr, "Unable to open file %s", filename);
        perror(NULL);
    }
    return fp;
}

fopen() isn't guaranteed to set errno, so I added a fprintf() call with the filename in addition to the perror() call. Also, it's a good idea to print the filename in your error messages about failing to open a file.

Alok