Hi

On Thu, Dec 15, 2016 at 4:40 AM, Michael Forney <mfor...@mforney.org> wrote:
> Previously, with -p, the specified directory and all of its parents
> would be 0777&~filemask (regardless of the -m flag). POSIX says parent
> directories must created as (0300|~filemask)&0777, and of course if -m
> is set, the specified directory should be created with those
> permissions.
>
> Additionally, POSIX says that for symbolic_mode strings, + and - should
> be interpretted relative to a default mode of 0777 (not 0).
>
> Without -p, previously the directory would be created first with
> 0777&~filemask (before a chmod), but POSIX says that the directory shall
> at no point in time have permissions less restrictive than the -m mode
> argument.
>
> Rather than dealing with mkdir removing the filemask bits by calling
> chmod afterward, just clear the umask and remove the bits manually.
> ---
>  libutil/mkdirp.c |  6 +++---
>  mkdir.c          | 19 ++++++++-----------
>  tar.c            |  2 +-
>  util.h           |  2 +-
>  4 files changed, 13 insertions(+), 16 deletions(-)

I have stared at the patch for a while and it removes more lines than
it adds so I think this one is okay.

Note that I did not have time to test it though.


Cheers,

Silvan


> diff --git a/libutil/mkdirp.c b/libutil/mkdirp.c
> index 2ef94a3..c3c678c 100644
> --- a/libutil/mkdirp.c
> +++ b/libutil/mkdirp.c
> @@ -7,7 +7,7 @@
>  #include "../util.h"
>
>  int
> -mkdirp(const char *path)
> +mkdirp(const char *path, mode_t mode, mode_t pmode)
>  {
>         char tmp[PATH_MAX], *p;
>         struct stat st;
> @@ -25,13 +25,13 @@ mkdirp(const char *path)
>                 if (*p != '/')
>                         continue;
>                 *p = '\0';
> -               if (mkdir(tmp, S_IRWXU | S_IRWXG | S_IRWXO) < 0 && errno != 
> EEXIST) {
> +               if (mkdir(tmp, pmode) < 0 && errno != EEXIST) {
>                         weprintf("mkdir %s:", tmp);
>                         return -1;
>                 }
>                 *p = '/';
>         }
> -       if (mkdir(tmp, S_IRWXU | S_IRWXG | S_IRWXO) < 0 && errno != EEXIST) {
> +       if (mkdir(tmp, mode) < 0 && errno != EEXIST) {
>                 weprintf("mkdir %s:", tmp);
>                 return -1;
>         }
> diff --git a/mkdir.c b/mkdir.c
> index 3e32d90..3e20b1a 100644
> --- a/mkdir.c
> +++ b/mkdir.c
> @@ -15,17 +15,18 @@ usage(void)
>  int
>  main(int argc, char *argv[])
>  {
> -       mode_t mode = 0, mask;
> -       int pflag = 0, mflag = 0, ret = 0;
> +       mode_t mode, mask;
> +       int pflag = 0, ret = 0;
> +
> +       mask = umask(0);
> +       mode = 0777 & ~mask;
>
>         ARGBEGIN {
>         case 'p':
>                 pflag = 1;
>                 break;
>         case 'm':
> -               mflag = 1;
> -               mask = getumask();
> -               mode = parsemode(EARGF(usage()), mode, mask);
> +               mode = parsemode(EARGF(usage()), 0777, mask);
>                 break;
>         default:
>                 usage();
> @@ -36,16 +37,12 @@ main(int argc, char *argv[])
>
>         for (; *argv; argc--, argv++) {
>                 if (pflag) {
> -                       if (mkdirp(*argv) < 0)
> +                       if (mkdirp(*argv, mode, 0777 & (~mask | 0300)) < 0)
>                                 ret = 1;
> -               } else if (mkdir(*argv, S_IRWXU | S_IRWXG | S_IRWXO) < 0) {
> +               } else if (mkdir(*argv, mode) < 0) {
>                         weprintf("mkdir %s:", *argv);
>                         ret = 1;
>                 }
> -               if (mflag && chmod(*argv, mode) < 0) {
> -                       weprintf("chmod %s:", *argv);
> -                       ret = 1;
> -               }
>         }
>
>         return ret;
> diff --git a/tar.c b/tar.c
> index d2161d4..f213039 100644
> --- a/tar.c
> +++ b/tar.c
> @@ -262,7 +262,7 @@ unarchive(char *fname, ssize_t l, char b[BLKSIZ])
>                 weprintf("remove %s:", fname);
>
>         tmp = estrdup(fname);
> -       mkdirp(dirname(tmp));
> +       mkdirp(dirname(tmp), 0777, 0777);
>         free(tmp);
>
>         switch (h->type) {
> diff --git a/util.h b/util.h
> index b5860dc..29325d2 100644
> --- a/util.h
> +++ b/util.h
> @@ -75,6 +75,6 @@ long long strtonum(const char *, long long, long long, 
> const char **);
>  long long enstrtonum(int, const char *, long long, long long);
>  long long estrtonum(const char *, long long, long long);
>  size_t unescape(char *);
> -int mkdirp(const char *);
> +int mkdirp(const char *, mode_t, mode_t);
>  #undef memmem
>  void *memmem(const void *, size_t, const void *, size_t);
> --
> 2.11.0
>
>

Reply via email to