#include <stdbool.h>
is better than typedef int bool;
#define true 1
#define false 0
Always, always, surround your if and else clauses with brackets. There are plenty of ways leaving them naked will screw you over. This goes for any language.This is terrifying, and broken:
char * s = malloc(snprintf(NULL, 0, "%s/%s", cwd, argv[1]) + 1);
sprintf(s, "%s/%s", cwd, argv[1]);
Passing NULL to snprintf, first of all, should be a huge red flag for you. If it's not, train yourself to recognize it. I realize what you're trying to do, but it relies on non-portable behavior. In particular, from the manual: The glibc implementation of the functions snprintf() and vsnprintf() conforms
to the C99 standard, that is, behaves as described above, since glibc version
2.1. Until glibc 2.0.6 they would return -1 when the output was truncated.
So until glibc 2.0.6, your program would be allocating 0 and then sprintfing into it. Always be careful when you read the manual, to read everything. And even then, program defensively, and train yourself to watch for errors like this one. In this case, passing NULL as the destination argument was an immediate no-no.In this function, you really should be checking errno (for ENOENT). stat(2) can fail for all sorts of reasons, one of them is that the entry is not there.
bool dirOrFileExists(const char dir[]) {
struct stat st;
if (stat(dir, &st) == 0) {
return true;
}
else {
return false;
}
}
Same for this loop, check errno to make sure the function's failing for the reason you think it should. while((entry = readdir(mydir)))
Here, you have something a little messy: int fileOrDirectory(const char path[]) {
struct stat s;
if (stat(path, &s) == 0) {
if (s.st_mode & S_IFDIR) {
// Is a directory
return 0;
} else if (s.st_mode & S_IFREG) {
// Is a file
return 1;
} else {
// Something else
return -1;
}
}
else {
// Error
printf("Error occurred\n");
}
}
First of all, returning values only you know about is bound for destruction. You should define an enum if you want a function to return specific states. Better yet, just do these checks in the function that would normally call this. It's not so bad usually, and you're probably calling stat(2) more than you should. It's a syscall, and those can be expensive. But more importantly, you have a branch where all you do is print something (to stdout, not stderr as you should), and then just return nothing! If you hit an error state, you need to either propagate that error up and handle it in the caller, or crash immediately. Don't leave your program in a garbage state and keep going. A compiler should have warned you about this (not having a return value in one branch). Turn on -Werror and at least -Wall. And fix your warnings, don't try to suppress them.You should use fscanf(3) instead of this:
while (1) {
ch = fgetc(fp);
newch[i] = ch;
if (ch == EOF) {
newch[i] = '\0';
break;
}
i++;
}
That's all for now, I gotta go. Good luck!