From 86737c5927c204dc866f1de7c95ab964e98be6af Mon Sep 17 00:00:00 2001 From: rilysh Date: Wed, 2 Oct 2024 21:40:06 +0530 Subject: [PATCH] Avoid computing strlen() inside loops Compiling with -O0 (no proper optimizations), strlen() call in loops for comparing the size, isn't being called/initialized before the actual loop gets started, which causes n-numbers of strlen() calls (as long as the string is). Keeping the length before entering in the loop is a good idea. On some places, even with -O2, both GCC and Clang can't recognize this pattern, which seem to happen in an array of char pointer. Reviewed-by: Brian Behlendorf Reviewed-by: Alexander Motin Signed-off-by: rilysh Closes #16584 --- cmd/zdb/zdb.c | 12 +++++++----- cmd/ztest.c | 8 +++++--- lib/libzfs/libzfs_util.c | 8 +++++--- module/zcommon/zfs_prop.c | 5 +++-- module/zcommon/zpool_prop.c | 5 +++-- udev/zvol_id.c | 7 +++++-- 6 files changed, 28 insertions(+), 17 deletions(-) diff --git a/cmd/zdb/zdb.c b/cmd/zdb/zdb.c index f185e2460..2c136bacb 100644 --- a/cmd/zdb/zdb.c +++ b/cmd/zdb/zdb.c @@ -8800,7 +8800,7 @@ zdb_read_block(char *thing, spa_t *spa) void *lbuf, *buf; char *s, *p, *dup, *flagstr, *sizes, *tmp = NULL; const char *vdev, *errmsg = NULL; - int i, error; + int i, len, error; boolean_t borrowed = B_FALSE, found = B_FALSE; dup = strdup(thing); @@ -8828,7 +8828,8 @@ zdb_read_block(char *thing, spa_t *spa) for (s = strtok_r(flagstr, ":", &tmp); s != NULL; s = strtok_r(NULL, ":", &tmp)) { - for (i = 0; i < strlen(flagstr); i++) { + len = strlen(flagstr); + for (i = 0; i < len; i++) { int bit = flagbits[(uchar_t)flagstr[i]]; if (bit == 0) { @@ -9098,13 +9099,14 @@ zdb_embedded_block(char *thing) static boolean_t zdb_numeric(char *str) { - int i = 0; + int i = 0, len; - if (strlen(str) == 0) + len = strlen(str); + if (len == 0) return (B_FALSE); if (strncmp(str, "0x", 2) == 0 || strncmp(str, "0X", 2) == 0) i = 2; - for (; i < strlen(str); i++) { + for (; i < len; i++) { if (!isxdigit(str[i])) return (B_FALSE); } diff --git a/cmd/ztest.c b/cmd/ztest.c index c58d35c99..523f280aa 100644 --- a/cmd/ztest.c +++ b/cmd/ztest.c @@ -685,15 +685,17 @@ static int str2shift(const char *buf) { const char *ends = "BKMGTPEZ"; - int i; + int i, len; if (buf[0] == '\0') return (0); - for (i = 0; i < strlen(ends); i++) { + + len = strlen(ends); + for (i = 0; i < len; i++) { if (toupper(buf[0]) == ends[i]) break; } - if (i == strlen(ends)) { + if (i == len) { (void) fprintf(stderr, "ztest: invalid bytes suffix: %s\n", buf); usage(B_FALSE); diff --git a/lib/libzfs/libzfs_util.c b/lib/libzfs/libzfs_util.c index cba071a1a..1f7e7b0e6 100644 --- a/lib/libzfs/libzfs_util.c +++ b/lib/libzfs/libzfs_util.c @@ -1618,15 +1618,17 @@ static int str2shift(libzfs_handle_t *hdl, const char *buf) { const char *ends = "BKMGTPEZ"; - int i; + int i, len; if (buf[0] == '\0') return (0); - for (i = 0; i < strlen(ends); i++) { + + len = strlen(ends); + for (i = 0; i < len; i++) { if (toupper(buf[0]) == ends[i]) break; } - if (i == strlen(ends)) { + if (i == len) { if (hdl) zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, "invalid numeric suffix '%s'"), buf); diff --git a/module/zcommon/zfs_prop.c b/module/zcommon/zfs_prop.c index 8179a1a8c..f7e6b41bf 100644 --- a/module/zcommon/zfs_prop.c +++ b/module/zcommon/zfs_prop.c @@ -808,11 +808,12 @@ zfs_name_to_prop(const char *propname) boolean_t zfs_prop_user(const char *name) { - int i; + int i, len; char c; boolean_t foundsep = B_FALSE; - for (i = 0; i < strlen(name); i++) { + len = strlen(name); + for (i = 0; i < len; i++) { c = name[i]; if (!zprop_valid_char(c)) return (B_FALSE); diff --git a/module/zcommon/zpool_prop.c b/module/zcommon/zpool_prop.c index afdbb6f15..d3355730b 100644 --- a/module/zcommon/zpool_prop.c +++ b/module/zcommon/zpool_prop.c @@ -494,11 +494,12 @@ vdev_name_to_prop(const char *propname) boolean_t vdev_prop_user(const char *name) { - int i; + int i, len; char c; boolean_t foundsep = B_FALSE; - for (i = 0; i < strlen(name); i++) { + len = strlen(name); + for (i = 0; i < len; i++) { c = name[i]; if (!zprop_valid_char(c)) return (B_FALSE); diff --git a/udev/zvol_id.c b/udev/zvol_id.c index 609349594..0d1da9956 100644 --- a/udev/zvol_id.c +++ b/udev/zvol_id.c @@ -56,6 +56,7 @@ main(int argc, const char *const *argv) return (1); } const char *dev_name = argv[1]; + size_t i, len; int fd; struct stat sb; @@ -73,11 +74,13 @@ main(int argc, const char *const *argv) } const char *dev_part = strrchr(dev_name, 'p'); + len = strlen(zvol_name); if (dev_part != NULL) { - sprintf(zvol_name + strlen(zvol_name), "-part%s", dev_part + 1); + sprintf(zvol_name + len, "-part%s", dev_part + 1); + len = strlen(zvol_name); } - for (size_t i = 0; i < strlen(zvol_name); ++i) + for (i = 0; i < len; ++i) if (isblank(zvol_name[i])) zvol_name[i] = '+';