On Fri, May 28, 2021 at 11:42:54PM +0200, Christian Weisgerber wrote:
> Todd C. Miller:
> 
> > Wouldn't it be simpler to use dprintf(3) instead of asprintf(3) +
> > write(2)?
> 
> Right, I keep forgetting that dprintf() exists.

Lovely. I have just one question below:

> diff 91b40e30e0dbff0c8a1546a02fb784fa8007a91b /home/naddy/got
> blob - b581bbe3e8fc97d4d3215bc948d761fa9c4135e9
> file + lib/fetch.c
> --- lib/fetch.c
> +++ lib/fetch.c
> @@ -156,8 +156,8 @@ dial_git(int *fetchfd, const char *host, const char *p
>  {
>  	const struct got_error *err = NULL;
>  	struct addrinfo hints, *servinfo, *p;
> -	char *cmd = NULL, *pkt = NULL;
> -	int fd = -1, totlen, r, eaicode;
> +	char *cmd = NULL;
> +	int fd = -1, len, r, eaicode;
>  
>  	*fetchfd = -1;
>  
> @@ -193,28 +193,12 @@ dial_git(int *fetchfd, const char *host, const char *p
>  		err = got_error_from_errno("asprintf");
>  		goto done;
>  	}
> -	totlen = 4 + strlen(cmd) + 1 + strlen("host=") + strlen(host) + 1;
> -	if (asprintf(&pkt, "%04x%s", totlen, cmd) == -1) {
> -		err = got_error_from_errno("asprintf");
> -		goto done;
> -	}
> -	r = write(fd, pkt, strlen(pkt) + 1);
> -	if (r == -1) {
> -		err = got_error_from_errno("write");
> -		goto done;
> -	}
> -	if (asprintf(&pkt, "host=%s", host) == -1) {
> -		err = got_error_from_errno("asprintf");
> -		goto done;
> -	}
> -	r = write(fd, pkt, strlen(pkt) + 1);
> -	if (r == -1) {
> -		err = got_error_from_errno("write");
> -		goto done;
> -	}
> +	len = 4 + strlen(cmd) + 1 + strlen("host=") + strlen(host) + 1;
> +	r = dprintf(fd, "%04x%s%chost=%s%c", len, cmd, '\0', host, '\0');
> +	if (r == -1)

dprintf's error indicator is defined as "less than 0" in the man page.
Should we check for r < 0 instead of r == -1?

> +		err = got_error_from_errno("dprintf");
>  done:
>  	free(cmd);
> -	free(pkt);
>  	if (err) {
>  		if (fd != -1)
>  			close(fd);
> -- 
> Christian "naddy" Weisgerber                          naddy@mips.inka.de
> 
>