Code:
int fexists(char *file)
{
struct stat *p;
p = malloc(sizeof(struct stat));
return(stat(file, p) == 0 && S_ISREG(p->st_mode));
}
Solution:
int fexists(char *file)
{
struct stat statbuf;
return(stat(file, &statbuf) == 0 && S_ISREG(statbuf.st_mode));
}
The second argument to stat() is a pointer, and that leads some beginning C programmers to feel that they need to declare a pointer variable. Some people do worse than the original code above in that they pass 'p' to stat() uninitialized, thus supplying an effectively random pointer value.
But stat() expects a pointer to an area where it can store all the stat data. The easiest way to create such an area is not malloc(), but a simple variable declaration.
This evades a bug in the original code, in that malloc() can return NULL if more memory isn't available; the original didn't check for that, so if malloc() returned NULL it would segfault in stat().
It fixes a subtler error in the original too: the original is a "memory leak" because each time you call it it does an additional malloc(). If this is in a long-running program (e.g. something like a web server) then it will eventually run out of memory. Malloc() is slow, and introduces a bookkeeping obligation (you have to make sure you free the memory eventually). Variable declarations are much quicker to execute, and are easier to write. Keep it simple!