On Tue, Jun 13, 2023 at 09:56:43PM +0200, Omar Polo wrote:
> got distracted by a thing and started to look at all strlen() calls (:
> 
> This one seems strange, s[strlen(s)] == '\0' should always be true,
> right?  I suppose the intent was to not skip remotes/foo/HEAD-xyz.

Yes, nice catch. ok.

Style-wise I prefer strcmp(..) == 0 over !strcmp(...) because I find == 0
easier to read and reason about. But ok either way.

> diff 33ff3adfa450b62d9bdfb406fb5d9f57a9da8a6e 6833fb12fde23540b00be57035e62a37a703ba33
> commit - 33ff3adfa450b62d9bdfb406fb5d9f57a9da8a6e
> commit + 6833fb12fde23540b00be57035e62a37a703ba33
> blob - f66d5b661f6d31bdbcb1027d5e6deaab89465001
> blob + 9ddcf2204a92d9d781114c1b05f5b946f98b26ab
> --- got/got.c
> +++ got/got.c
> @@ -4101,7 +4101,7 @@ build_refs_str(char **refs_str, struct got_reflist_hea
>  				continue;
>  			name += 8;
>  			s = strstr(name, "/" GOT_REF_HEAD);
> -			if (s != NULL && s[strlen(s)] == '\0')
> +			if (s != NULL && !strcmp(s, "/" GOT_REF_HEAD))
>  				continue;
>  		}
>  		err = got_ref_resolve(&ref_id, repo, re->ref);
> blob - bd2b583446e8eb32ab6649c2c1316ab6da2f1b6e
> blob + 56083f9e2092c3282a6f4e217890ca9ed534e5a9
> --- tog/tog.c
> +++ tog/tog.c
> @@ -2342,7 +2342,7 @@ build_refs_str(char **refs_str, struct got_reflist_hea
>  		if (strncmp(name, "remotes/", 8) == 0) {
>  			name += 8;
>  			s = strstr(name, "/" GOT_REF_HEAD);
> -			if (s != NULL && s[strlen(s)] == '\0')
> +			if (s != NULL && !strcmp(s, "/" GOT_REF_HEAD))
>  				continue;
>  		}
>  		err = got_ref_resolve(&ref_id, repo, re->ref);
> 
>