On 23-06-15 04:57PM, Omar Polo wrote:
> 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(-)

I actually much prefer this, too, as it is more readable imo. Debatable
performance gain notwithstanding, this is an improvement so ok for me.

Thanks for taking the time to extend the sweep through the rest of the
codebase :)

> 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] = '?';
>  			}
> 

-- 
Mark Jamsek <fnc.bsdbox.org|got.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68