I wrote a program that creates a directory, similar to the mkdir command.
My question is, is this implementation correct? What else could I improve or what problems does implementation have?
code:
#include <stdio.h>
#include <stdlib.h>
#include <fcntl.h>
#include <unistd.h>
#include <sys/stat.h>
#include <stdbool.h>
#include "lab.h"
int main(int argc, char* argv[])
{
bool err = false;
if(argc == 1)
{
err_msg("missing operand\nTry 'mkdir --help' for more information.");
exit(1);
}
for(int i=1; i<argc; i++)
{
if(access(argv[i],F_OK) == 0)
{
err_msg("cannot create directory ‘%s’: File exists",argv[i]);
err = true;
}
else
{
if(mkdir(argv[i], S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH) == -1)
{
err_ret("cannot create directory '%s'",argv[i]);
exit(1);
}
}
}
if(err == false)
exit(0);
else
exit(1);
}
lab.h content:
/* Our own header, to be included *after* all standard system headers */
#ifndef __LAB_H__
#define __LAB_H__
#include <sys/types.h> /* required for some of our prototypes */
#include <stdio.h> /* for convenience */
#include <stdlib.h> /* for convenience */
#include <string.h> /* for convenience */
#include <unistd.h> /* for convenience */
#define MAXLINE 4096 /* max line length */
#define FILE_MODE (S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH)
/* default file access permissions for new files */
#define DIR_MODE (FILE_MODE | S_IXUSR | S_IXGRP | S_IXOTH)
/* default permissions for new directories */
/* prototypes for our own functions */
char *path_alloc(int *); /* {Prog pathalloc} */
int open_max(void); /* {Prog openmax} */
void clr_fl(int, int); /* {Prog setfl} */
void set_fl(int, int); /* {Prog setfl} */
void err_dump(const char *, ...); /* {App misc_source} */
void err_msg(const char *, ...);
void err_quit(const char *, ...);
void err_ret(const char *, ...);
void err_sys(const char *, ...);
#endif /* __LAB_H__ */
```
-
1\$\begingroup\$ And what about implementation of lab.h? \$\endgroup\$convert– convert2022年02月06日 00:03:13 +00:00Commented Feb 6, 2022 at 0:03
-
\$\begingroup\$ It's a header that must be in implementation \$\endgroup\$Mark– Mark2022年02月06日 09:47:20 +00:00Commented Feb 6, 2022 at 9:47
-
\$\begingroup\$ I know, so I was asking about coresponding c file. \$\endgroup\$convert– convert2022年02月06日 11:20:17 +00:00Commented Feb 6, 2022 at 11:20
1 Answer 1
Don't call access()
Calling access() before trying mkdir() results in a TOCTTOU bug. Some other process might create or delete the directory you are trying to mkdir() between the call to access() and mkdir(). Instead, just call mkdir() and check the value of errno to see if there was an error because the path already existed or if there was another reason:
if (mkdir(argv[1], ...) == -1) {
err_ret("cannot create directory '%s'",argv[i]);
if (errno == EEXIST) {
err = true;
} else {
exit(1);
}
}
Don't exit immediately on error
The mkdir command will continue trying to make all specified directories, regardless of what kind of error occurs. So I would just set err = true in all cases, and not call exit(1) inside the for-loop.
Consider using the BSD err() function
Instead of implementing your own err_*() functions, consider using err(), warn() and related functions from BSD. These are supported on BSD, Linux and macOS. If you want to support Windows as well, I suggest implementing your own version of them and conditionally compile them only on Windows.