Copy a directory from source to destination

I wrote a program that copies a directory from a source to a destination, similar to the cp utility.

For this, I used the essential functions:

open(), write(), mkdir(), readdir().

This is the source code (ignore the main() function, this is not the final form):

```
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include

int isDirectory(char*);
int copyFile(char, char);
int copyDirectory(char, char);

int main(int argc, char **argv) {

copyDirectory((argv + 1), (argv + 2));

return 0;
}

int isDirectory(char *path) {
struct stat statbuf;

if (stat(path, &statbuf) == -1) {
return -1;
}
else {
return S_ISDIR(statbuf.st_mode);
}
}

int copyFile(char source, char destination) {
if((source == NULL) || (destination == NULL)) {
return -1;
}

struct stat statbuf;
int fdr, fdw, rv;
char buff[256];

stat(source, &statbuf);
if(!S_ISREG(statbuf.st_mode)) {
return 1;
}

fdr = open(source, O_RDONLY);
fdw = open(destination, O_CREAT|O_WRONLY, S_IREAD|S_IWRITE);

while((rv = read(fdr, &buff, 128))) {
write(fdw, &buff, rv);
}

return 0;
}

int copyDirectory(char source, char destination) {
if((source == NULL) || (destination == NULL)) {
return -1;
}

struct dirent *entry;
DIR *dr;
char sourcePatch[512], destinationPatch[512];

strcpy(sourcePatch, source);
strcpy(destinationPatch, destination);
strcat(destinationPatch, "/");
strcat(sourcePatch, "/");

if(!isDirectory(source)) {
return -1;
}

if(opendir(destination) == NULL) {
mkdir(destination, 0700);
}

dr = opendir(source);
while((entry = readdir(dr))) {
strcat(sourcePatch, entry->d_name);
strcat(destinationPatch, entry->d_name); //printf("%s\n", sourcePatch);

if(isDirectory(sourcePatch) && strcmp(entry->d_name, ".") && strcmp(entry->d_name, "..")) {
copyDirectory(sourcePatch, destinationPatch);
}
else {
copyFile(sourcePa

Overall, it is good, but I would apply the following changes:

-
Use const for read only function parameters. All the char pointers you are passing to the functions are actually only read from, so you should use const char to document that.

-
Personally, I'm not a big fan of returning -1, 0, 1, etc. This kind of error codes are popular with UNIX system calls, but I find them rather error prone. It is easy to make a mistake with the return value of a function if you don't carefully read the documentation. If the function has only two possible outcomes, success and failure, then include ` and return true or false. There is no chance of misunderstanding it this way.

-
A few magic numbers that could be named constants. For example, why did you choose 512 and 256 for buffer sizes? That is not obvious. Named constants would be better for these cases. Also, even though the 0700 constant on mkdir is well known for UNIX users, it might not be so for others less familiar with the architecture. I would also make that a named constant.

-
I suggest keeping the parameter names on function prototypes. In int copyFile(char, char); which one is the source file and which is the dest? Can't tell for sure just by reading the function prototype. Parameter names would make that clear without having to look at the implementation.

-
I suspect you have too many #includes for such a tiny program. ` is certainly not used here. Make sure to only import the dependencies that your program actually requires.

-
A few system calls are not being checked for failure, so your program is not as robust as it could be.

StackExchange Code Review Q#70119, answer score: 2