[hackers] [sbase] mkfifo: Simplify -m handling || Michael Forney

[git]

commit 01b5105e0c7439a358d095fe4703e88c31095178
Author: Michael Forney 
AuthorDate: Thu Jun 13 13:45:37 2019 -0700
Commit: Michael Forney 
CommitDate: Thu Jun 13 13:45:37 2019 -0700

mkfifo: Simplify -m handling

Rather than create the FIFO with incorrect permissions at first, then
restore with chmod(2), just clear the umask when -m is specified, and
pass the parsed mode to mkfifo(2).

diff --git a/mkfifo.c b/mkfifo.c
index 390381b..2470a09 100644
--- a/mkfifo.c
+++ b/mkfifo.c
@@ -14,14 +14,12 @@ usage(void)
 int
 main(int argc, char *argv[])
 {
-   mode_t mode = 0666, mask;
-   int mflag = 0, ret = 0;
+   mode_t mode = 0666;
+   int ret = 0;
 
ARGBEGIN {
case 'm':
-   mflag = 1;
-   mask = getumask();
-   mode = parsemode(EARGF(usage()), mode, mask);
+   mode = parsemode(EARGF(usage()), mode, umask(0));
break;
default:
usage();
@@ -31,15 +29,9 @@ main(int argc, char *argv[])
usage();
 
for (; *argv; argc--, argv++) {
-   if (mkfifo(*argv, S_IRUSR | S_IWUSR | S_IRGRP |
-   S_IWGRP | S_IROTH | S_IWOTH) < 0) {
+   if (mkfifo(*argv, mode) < 0) {
weprintf("mkfifo %s:", *argv);
ret = 1;
-   } else if (mflag) {
-   if (chmod(*argv, mode) < 0) {
-   weprintf("chmod %s:", *argv);
-   ret = 1;
-   }
}
}
 

Re: [hackers] [sbase] [PATCH] mkfifo: Simplify -m handling

[Richard Ipsum]

On Thu, Jun 13, 2019 at 05:27:37PM -0700, Michael Forney wrote:
> Thanks for looking this over.
> 
> On 2019-06-13, Richard Ipsum  wrote:
> > I might be wrong but I think this will cause a subtle change in
> > behaviour since the umask is needed to correctly interpret the mode
> > string. One example that's documented in chmod(1p) is "-w" vs "a-w".
> > "a-w" removes all write perms, but "-w" removes only the permissions
> > that weren't filtered by the mask. This could explain why this
> > is currently being done in two separate steps.
> 
> umask(0) sets the mask to 0 and returns the old mask, so I think the
> mode is interpreted the same way before and after this change
> (getumask() is even implemented as two calls to umask, returning the
> result of the first call).

Ah right, I read this thinking the mask passed to the mode string
parser would be 0, but obviously it's not. So you pass the existing
mode while setting the mask to 0 so that you can use the final mode
as the argument to mkfifo, and get rid of the chmod call, nice.

Thanks,
Richard