From 107932aefd1ee5664d77b585655e918656c4e766 Mon Sep 17 00:00:00 2001 From: rilysh Date: Mon, 30 Sep 2024 23:26:04 +0530 Subject: [PATCH 1/3] 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. Signed-off-by: rilysh --- cmd/zdb/zdb.c | 11 ++++++----- 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 | 6 ++++-- 6 files changed, 26 insertions(+), 17 deletions(-) diff --git a/cmd/zdb/zdb.c b/cmd/zdb/zdb.c index 8e3b6972ae04..6c2865cdb32a 100644 --- a/cmd/zdb/zdb.c +++ b/cmd/zdb/zdb.c @@ -8788,7 +8788,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); @@ -8816,7 +8816,7 @@ 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++) { + for (i = 0, len = strlen(flagstr); i < len; i++) { int bit = flagbits[(uchar_t)flagstr[i]]; if (bit == 0) { @@ -9086,13 +9086,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 c58d35c9909c..523f280aae1a 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 cba071a1a900..1f7e7b0e647e 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 20cc0dffc27e..5718a6de4f3f 100644 --- a/module/zcommon/zfs_prop.c +++ b/module/zcommon/zfs_prop.c @@ -804,11 +804,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 afdbb6f15e97..d3355730ba3d 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 609349594767..1374d7dddfa5 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,12 @@ 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); } - for (size_t i = 0; i < strlen(zvol_name); ++i) + for (i = 0; i < len; ++i) if (isblank(zvol_name[i])) zvol_name[i] = '+'; From aa9e3daf4b044325fd5aacc47401ed8e459e1f63 Mon Sep 17 00:00:00 2001 From: rilysh Date: Mon, 30 Sep 2024 23:53:37 +0530 Subject: [PATCH 2/3] zvol_id.c: Compute the new length Since zvol_name was modified, we need to compute the length again. Signed-off-by: rilysh --- udev/zvol_id.c | 1 + 1 file changed, 1 insertion(+) diff --git a/udev/zvol_id.c b/udev/zvol_id.c index 1374d7dddfa5..0d1da9956749 100644 --- a/udev/zvol_id.c +++ b/udev/zvol_id.c @@ -77,6 +77,7 @@ main(int argc, const char *const *argv) len = strlen(zvol_name); if (dev_part != NULL) { sprintf(zvol_name + len, "-part%s", dev_part + 1); + len = strlen(zvol_name); } for (i = 0; i < len; ++i) From 4b1c8964ca5d1e769b69064f76e99c7b72c5002d Mon Sep 17 00:00:00 2001 From: rilysh Date: Tue, 1 Oct 2024 22:09:06 +0530 Subject: [PATCH 3/3] zdb.c: move strlen() before the for loop Signed-off-by: rilysh --- cmd/zdb/zdb.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/zdb/zdb.c b/cmd/zdb/zdb.c index 6c2865cdb32a..033947a898ce 100644 --- a/cmd/zdb/zdb.c +++ b/cmd/zdb/zdb.c @@ -8816,7 +8816,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, len = strlen(flagstr); i < len; i++) { + len = strlen(flagstr); + for (i = 0; i < len; i++) { int bit = flagbits[(uchar_t)flagstr[i]]; if (bit == 0) {