2
\$\begingroup\$

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__ */
```
\$\endgroup\$
3
  • 1
    \$\begingroup\$ And what about implementation of lab.h? \$\endgroup\$
    – convert
    Feb 6, 2022 at 0:03
  • \$\begingroup\$ It's a header that must be in implementation \$\endgroup\$
    – Mark
    Feb 6, 2022 at 9:47
  • \$\begingroup\$ I know, so I was asking about coresponding c file. \$\endgroup\$
    – convert
    Feb 6, 2022 at 11:20

1 Answer 1

4
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.