From: Omar Polo Subject: avoid some gratious strlen() To: gameoftrees@openbsd.org Date: Thu, 15 Jun 2023 16:57:30 +0200 last part of my quest. hope not to annoy, but i get slightly triggered by strlen(foo) == 0 since it need to traverse the whole string just to assert that the first byte is not NUL. After seeing it a few times in gotwebd i decided to do a sweep, and that explains my recent diffs. Unlinke the previous, this is highly subjective and opinable. It avoids computing the string length just to see if it's empty, and changes some for() loops too along the way. I haven't checked, but won't be surprised if the compiler emits the same code before and after the diff. I'd still prefer without all these strlen(), and I personally find it even easier to read, but I'm not attached to it. I can't play the optimization card since these places are nowhere near being performance-critical. since i wrote i figured out i'd send it. now back to real work ;) diffstat refs/heads/main refs/heads/strlen M got/got.c | 5+ 5- M gotwebd/config.c | 1+ 1- M gotwebd/got_operations.c | 2+ 2- M gotwebd/gotweb.c | 4+ 4- M gotwebd/sockets.c | 1+ 1- M tog/tog.c | 1+ 1- 6 files changed, 14 insertions(+), 14 deletions(-) diff refs/heads/main refs/heads/strlen commit - f4a5cef1546205afab47f148edefabcf77c06d3b commit + bedba0b194efdbd9ade4ce8005b575a8ad74b24c blob - 94bf306447c1f76eadeb7357e88c4c7b6fce7790 blob + b713aa2e5f2e3920e0055fb2366825c0e1487eff --- got/got.c +++ got/got.c @@ -878,7 +878,7 @@ cmd_import(int argc, char *argv[]) * unveil(2) traverses exec(2); if an editor is used we have * to apply unveil after the log message has been written. */ - if (logmsg == NULL || strlen(logmsg) == 0) { + if (logmsg == NULL || *logmsg == '\0') { error = get_editor(&editor); if (error) goto done; @@ -5969,7 +5969,7 @@ print_entry(struct got_tree_entry *te, const char *id, err = got_tree_entry_get_symlink_target(&link_target, te, repo); if (err) return err; - for (i = 0; i < strlen(link_target); i++) { + for (i = 0; link_target[i] != '\0'; i++) { if (!isprint((unsigned char)link_target[i])) link_target[i] = '?'; } @@ -6326,7 +6326,7 @@ cmd_status(int argc, char *argv[]) st.suppress = 1; /* fallthrough */ case 's': - for (i = 0; i < strlen(optarg); i++) { + for (i = 0; optarg[i] != '\0'; i++) { switch (optarg[i]) { case GOT_STATUS_MODIFY: case GOT_STATUS_ADD: @@ -7989,7 +7989,7 @@ cmd_remove(int argc, char *argv[]) can_recurse = 1; break; case 's': - for (i = 0; i < strlen(optarg); i++) { + for (i = 0; optarg[i] != '\0'; i++) { switch (optarg[i]) { case GOT_STATUS_MODIFY: delete_local_mods = 1; @@ -8915,7 +8915,7 @@ collect_commit_logmsg(struct got_pathlist_head *commit size_t len; /* if a message was specified on the command line, just use it */ - if (a->cmdline_log != NULL && strlen(a->cmdline_log) != 0) { + if (a->cmdline_log != NULL && *a->cmdline_log != '\0') { len = strlen(a->cmdline_log) + 1; *logmsg = malloc(len + 1); if (*logmsg == NULL) blob - b13f0b45ecb8c13c605728f061a869d69085a4e0 blob + 1c8057cb99a030e7a3b45871f35124b82101e918 --- gotwebd/config.c +++ gotwebd/config.c @@ -228,7 +228,7 @@ config_getsock(struct gotwebd *env, struct imsg *imsg) sock->conf.af_type == AF_UNIX ? "unix" : (sock->conf.af_type == AF_INET ? "inet" : (sock->conf.af_type == AF_INET6 ? "inet6" : "unknown")), - strlen(sock->conf.unix_socket_name) ? + *sock->conf.unix_socket_name != '\0' ? sock->conf.unix_socket_name : "none"); return 0; blob - 61bee36696a4edd811768fba90d50e909418b91f blob + a7525287e2c962f40938fd909cdb0b9142d1f605 --- gotwebd/got_operations.c +++ gotwebd/got_operations.c @@ -342,7 +342,7 @@ got_get_repo_commits(struct request *c, size_t limit) TAILQ_INIT(&refs); - if (qs->file != NULL && strlen(qs->file) > 0) + if (qs->file != NULL && *qs->file != '\0') if (asprintf(&file_path, "%s/%s", qs->folder ? qs->folder : "", qs->file) == -1) return got_error_from_errno("asprintf"); @@ -376,7 +376,7 @@ got_get_repo_commits(struct request *c, size_t limit) if (error) goto done; - if (qs->file != NULL && strlen(qs->file) > 0) { + if (qs->file != NULL && *qs->file != '\0') { error = got_commit_graph_open(&graph, file_path, 0); if (error) goto done; blob - 983fb20185d96176ed732dfe115d8e8dd355b575 blob + 57acfb3983fd313a2a166e1c1296ed5262cb5b3d --- gotwebd/gotweb.c +++ gotwebd/gotweb.c @@ -392,13 +392,13 @@ gotweb_get_server(uint8_t *server_name, uint8_t *subdo struct server *srv = NULL; /* check against the server name first */ - if (strlen(server_name) > 0) + if (*server_name != '\0') TAILQ_FOREACH(srv, &gotwebd_env->servers, entry) if (strcmp(srv->name, server_name) == 0) goto done; /* check against subdomain second */ - if (strlen(subdomain) > 0) + if (*subdomain != '\0') TAILQ_FOREACH(srv, &gotwebd_env->servers, entry) if (strcmp(srv->name, subdomain) == 0) goto done; @@ -614,7 +614,7 @@ qa_found: } break; case INDEX_PAGE: - if (strlen(value) == 0) + if (*value == '\0') break; (*qs)->index_page = strtonum(value, INT64_MIN, INT64_MAX, &errstr); @@ -635,7 +635,7 @@ qa_found: } break; case PAGE: - if (strlen(value) == 0) + if (*value == '\0') break; (*qs)->page = strtonum(value, INT64_MIN, INT64_MAX, &errstr); blob - bf53d6f9bff50c34f85de2733fe7ae29ff2401d7 blob + 3f932978bf05a203c0c4bcd43bf7390fb4d153c8 --- gotwebd/sockets.c +++ gotwebd/sockets.c @@ -231,7 +231,7 @@ sockets_conf_new_socket_fcgi(struct gotwebd *env, stru memcpy(&acp->ss, &a->ss, sizeof(acp->ss)); acp->ipproto = a->ipproto; acp->port = a->port; - if (strlen(a->ifname) != 0) { + if (*a->ifname != '\0') { if (strlcpy(acp->ifname, a->ifname, sizeof(acp->ifname)) >= sizeof(acp->ifname)) { fatalx("%s: interface name truncated", blob - d694a6164057730509b8e3db4e24c4ae1ef16c64 blob + 50c38cd6d28d539dcfa273bc3233bbda7f02fe7a --- tog/tog.c +++ tog/tog.c @@ -7304,7 +7304,7 @@ draw_tree_entries(struct tog_view *view, const char *p free(id_str); return err; } - for (i = 0; i < strlen(link_target); i++) { + for (i = 0; i < link_target[i] != '\0'; i++) { if (!isprint((unsigned char)link_target[i])) link_target[i] = '?'; }