Cleanup the os_dirpath logic #2756
Conversation
… actually gets created (regardless of what mkdir returns), and pretty-prints the error if it does error out. Signed-off-by: Ralph Castain <rhc@open-mpi.org> (cherry picked from commit b257c32)
|
Awesome, thanks! You guys sure do respond quickly. One thing though - with the current patch, I think the stat() might destroy the errno from mkdir(), in which case the message from strerror will end up being wrong. |
|
crumby - you are quite correct. will fix. thanks! |
…l likely overwrite it Signed-off-by: Ralph Castain <rhc@open-mpi.org> (cherry picked from commit 6da4dbb)
| /* Now that we have the name, try to create it */ | ||
| mkdir(tmp, mode); | ||
| ret = errno; // save the errno for an error msg, if needed | ||
| if (0 != stat(tmp, &buf)) { |
There was a problem hiding this comment.
the pointed commit was meant to solve a race condition (e.g. mkdir() after stat())
there still is a tricky race condition here ... someone else might have mkdir() but with the wrong permissions, so it will likely crash later.
so i guess we have to check the permissions at the end.
this is for master
diff --git a/opal/util/os_dirpath.c b/opal/util/os_dirpath.c
index 9ef9ebb..2328179 100644
--- a/opal/util/os_dirpath.c
+++ b/opal/util/os_dirpath.c
@@ -9,7 +9,7 @@
* University of Stuttgart. All rights reserved.
* Copyright (c) 2004-2005 The Regents of the University of California.
* All rights reserved.
- * Copyright (c) 2015-2016 Research Organization for Information Science
+ * Copyright (c) 2015-2017 Research Organization for Information Science
* and Technology (RIST). All rights reserved.
* Copyright (c) 2016-2017 Intel, Inc. All rights reserved.
* $COPYRIGHT$
@@ -121,6 +121,12 @@ int opal_os_dirpath_create(const char *path, const mode_t mode)
opal_argv_free(parts);
free(tmp);
return OPAL_ERROR;
+ } else if (i == (len-1) && (mode != (mode & buf.st_mode)) && (0 > chmod(tmp, (buf.st_mode | mode)))) {
+ opal_show_help("help-opal-util.txt", "dir-mode", true,
+ tmp, mode, strerror(errno));
+ opal_argv_free(parts);
+ free(tmp);
+ return(OPAL_ERR_PERM); /* can't set correct mode */
}
}
There was a problem hiding this comment.
sounds reasonable - if you want to commit to master, I'll cherry-pick it into this PR
There was a problem hiding this comment.
ok, i made #2759
will merge when CI completes
always check the permissions of the created directory, in case some one else created the very same directory but with incompatible permissions Signed-off-by: Gilles Gouaillardet <gilles@rist.or.jp> (cherry picked from commit dffaad9)
|
@ggouaillardet Thanks! Cherry-picked your update so it is included here |
|
@hppritcha This is good to go when CI finishes. It fixes an issue on Lustre. |
|
Hi all, Just a heads up, I hit this again on v2.1.0 - it appears to not have been cherrypicked into v2.x. Cheers, |
|
Thanks! |
Cleanup the os_dirpath logic so it doesn't error out if the directory actually gets created (regardless of what mkdir returns), and pretty-prints the error if it does error out.
Fixes #2754
Signed-off-by: Ralph Castain rhc@open-mpi.org
(cherry picked from commit b257c32)