From db9048741ba4ccad874218ef8b36b27192aada0b Mon Sep 17 00:00:00 2001 From: Colm Date: Thu, 3 Jun 2021 16:13:42 +0100 Subject: [PATCH] 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 Reviewed-by: John Kennedy Signed-off-by: Colm Buckley Closes #12156 --- lib/libzfs/libzfs_pool.c | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/lib/libzfs/libzfs_pool.c b/lib/libzfs/libzfs_pool.c index d4849ee5bd..310352ce1f 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);