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 <behlendorf1@llnl.gov>
Reviewed-by: John Kennedy <john.kennedy@delphix.com>
Signed-off-by: Colm Buckley <colm@tuatha.org>
Closes #12156
This commit is contained in:
Colm 2021-06-03 16:13:42 +01:00 committed by GitHub
parent f645d4416f
commit f97142c748
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 24 additions and 12 deletions

View File

@ -4796,11 +4796,16 @@ zpool_load_compat(const char *compat, boolean_t *features, char *report,
* as they're only needed if the filename is relative * as they're only needed if the filename is relative
* which will be checked during the openat(). * 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 #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); (void) strlcpy(l_compat, compat, ZFS_MAXPROPLEN);
@ -4808,7 +4813,7 @@ zpool_load_compat(const char *compat, boolean_t *features, char *report,
file != NULL; file != NULL;
file = strtok_r(NULL, ",", &ps)) { 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; enum { Z_SYSCONF, Z_DATA } source;
@ -4831,14 +4836,18 @@ zpool_load_compat(const char *compat, boolean_t *features, char *report,
continue; continue;
} }
#if !defined(MAP_POPULATE) && defined(MAP_PREFAULT_READ) /* Prefault the file if system allows */
#define MAP_POPULATE MAP_PREFAULT_READ #if defined(MAP_POPULATE)
#elif !defined(MAP_POPULATE) #define ZC_MMAP_FLAGS (MAP_PRIVATE | MAP_POPULATE)
#define MAP_POPULATE 0 #elif defined(MAP_PREFAULT_READ)
#define ZC_MMAP_FLAGS (MAP_PRIVATE | MAP_PREFAULT_READ)
#else
#define ZC_MMAP_FLAGS (MAP_PRIVATE)
#endif #endif
/* private mmap() so we can strtok safely */ /* private mmap() so we can strtok safely */
fc = (char *)mmap(NULL, fs.st_size, fc = (char *)mmap(NULL, fs.st_size, PROT_READ | PROT_WRITE,
PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_POPULATE, featfd, 0); ZC_MMAP_FLAGS, featfd, 0);
(void) close(featfd); (void) close(featfd);
/* map ok, and last character == newline? */ /* map ok, and last character == newline? */
@ -4852,7 +4861,10 @@ zpool_load_compat(const char *compat, boolean_t *features, char *report,
ret_nofiles = B_FALSE; 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'; fc[fs.st_size - 1] = '\0';
for (line = strtok_r(fc, "\n", &ls); for (line = strtok_r(fc, "\n", &ls);