A couple of small style cleanups

In `zpool_load_compat()`:

  * initialize `l_features[]` with a loop rather than a static
    initializer.

  * don't redefine system constants; use private names instead

Rationale here:

When an array is initialized using a static {foo}, only the specified
members are initialized to the provided values, the rest are
initialized to zero. While B_FALSE is of course zero, it feels
unsafe to rely on this being true forever, so I'm inclined to sacrifice
a few microseconds of runtime here and initialize using a loop.

When looking for the correct combination of system constants to use
(in open() and mmap()), I prefer to use private constants rather than
redefining system ones; due to the small chance that the system
ones might be referenced later in the file. So rather than defining
O_PATH and MAP_POPULATE, I use distinct constant names.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: John Kennedy <[email protected]>
Signed-off-by: Colm Buckley <[email protected]>
Closes #12156

1 files changed, 24 insertions, 12 deletions

diff --git a/lib/libzfs/libzfs_pool.c b/lib/libzfs/libzfs_pool.c
index d4849ee5b..310352ce1 100644
--- a/lib/libzfs/libzfs_pool.c
+++ b/lib/libzfs/libzfs_pool.c
@@ -4800,11 +4800,16 @@ zpool_load_compat(const char *compat, boolean_t *features, char *report,
 	 * as they're only needed if the filename is relative
 	 * which will be checked during the openat().
 	 */
-#ifndef O_PATH
-#define	O_PATH O_RDONLY
+
+/* O_PATH safer than O_RDONLY if system allows it */
+#if defined(O_PATH)
+#define	ZC_DIR_FLAGS (O_DIRECTORY | O_CLOEXEC | O_PATH)
+#else
+#define	ZC_DIR_FLAGS (O_DIRECTORY | O_CLOEXEC | O_RDONLY)
 #endif
-	sdirfd = open(ZPOOL_SYSCONF_COMPAT_D, O_DIRECTORY | O_PATH | O_CLOEXEC);
-	ddirfd = open(ZPOOL_DATA_COMPAT_D, O_DIRECTORY | O_PATH | O_CLOEXEC);
+
+	sdirfd = open(ZPOOL_SYSCONF_COMPAT_D, ZC_DIR_FLAGS);
+	ddirfd = open(ZPOOL_DATA_COMPAT_D, ZC_DIR_FLAGS);
 
 	(void) strlcpy(l_compat, compat, ZFS_MAXPROPLEN);
 
@@ -4812,7 +4817,7 @@ zpool_load_compat(const char *compat, boolean_t *features, char *report,
 	    file != NULL;
 	    file = strtok_r(NULL, ",", &ps)) {
 
-		boolean_t l_features[SPA_FEATURES] = {B_FALSE};
+		boolean_t l_features[SPA_FEATURES];
 
 		enum { Z_SYSCONF, Z_DATA } source;
 
@@ -4835,14 +4840,18 @@ zpool_load_compat(const char *compat, boolean_t *features, char *report,
 			continue;
 		}
 
-#if !defined(MAP_POPULATE) && defined(MAP_PREFAULT_READ)
-#define	MAP_POPULATE MAP_PREFAULT_READ
-#elif !defined(MAP_POPULATE)
-#define	MAP_POPULATE 0
+/* Prefault the file if system allows */
+#if defined(MAP_POPULATE)
+#define	ZC_MMAP_FLAGS (MAP_PRIVATE | MAP_POPULATE)
+#elif defined(MAP_PREFAULT_READ)
+#define	ZC_MMAP_FLAGS (MAP_PRIVATE | MAP_PREFAULT_READ)
+#else
+#define	ZC_MMAP_FLAGS (MAP_PRIVATE)
 #endif
+
 		/* private mmap() so we can strtok safely */
-		fc = (char *)mmap(NULL, fs.st_size,
-		    PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_POPULATE, featfd, 0);
+		fc = (char *)mmap(NULL, fs.st_size, PROT_READ | PROT_WRITE,
+		    ZC_MMAP_FLAGS, featfd, 0);
 		(void) close(featfd);
 
 		/* map ok, and last character == newline? */
@@ -4856,7 +4865,10 @@ zpool_load_compat(const char *compat, boolean_t *features, char *report,
 
 		ret_nofiles = B_FALSE;
 
-		/* replace last char with NUL to ensure we have a delimiter */
+		for (uint_t i = 0; i < SPA_FEATURES; i++)
+			l_features[i] = B_FALSE;
+
+		/* replace final newline with NULL to ensure string ends */
 		fc[fs.st_size - 1] = '\0';
 
 		for (line = strtok_r(fc, "\n", &ls);