From: Tracey Emery <tracey@traceyemery.net>
Subject: Re: Only use string literals as format strings for dprintf()
To: Christian Weisgerber <naddy@mips.inka.de>
Cc: gameoftrees@openbsd.org
Date: Thu, 10 Sep 2020 15:31:35 -0600

On Thu, Sep 10, 2020 at 11:04:20PM +0200, Christian Weisgerber wrote:
> Stefan Sperling:
> 
> > And I agree that write(2) would be a better choice.
> 
> Like this?
> 
> diff 0823ffc2f6c509dbcedfb15d0d1011a253b45ef9 /home/naddy/got
> blob - 3a7013aa09b5952fd99ebb4cbf7e06235e769d42
> file + got/got.c
> --- got/got.c
> +++ got/got.c
> @@ -486,12 +486,13 @@ collect_import_msg(char **logmsg, char **logmsg_path, 
>      const char *path_dir, const char *branch_name)
>  {
>  	char *initial_content = NULL;
> +	size_t initial_content_len;
>  	const struct got_error *err = NULL;
>  	int fd;
>  
> -	if (asprintf(&initial_content,
> +	if ((initial_content_len = asprintf(&initial_content,
>  	    "\n# %s to be imported to branch %s\n", path_dir,
> -	    branch_name) == -1)

Assignments in a conditional is going to give stsp a coronary, or at
the least, make his right eye agressively twitch! ;)

> +	    branch_name)) == -1)
>  		return got_error_from_errno("asprintf");
>  
>  	err = got_opentemp_named_fd(logmsg_path, &fd,
> @@ -499,7 +500,7 @@ collect_import_msg(char **logmsg, char **logmsg_path, 
>  	if (err)
>  		goto done;
>  
> -	dprintf(fd, initial_content);
> +	write(fd, initial_content, initial_content_len);
>  	close(fd);
>  
>  	err = edit_logmsg(logmsg, editor, *logmsg_path, initial_content);
> @@ -5643,6 +5644,7 @@ get_tag_message(char **tagmsg, char **tagmsg_path, con
>  	const struct got_error *err = NULL;
>  	char *template = NULL, *initial_content = NULL;
>  	char *editor = NULL;
> +	size_t initial_content_len;
>  	int fd = -1;
>  
>  	if (asprintf(&template, GOT_TMPDIR_STR "/got-tagmsg") == -1) {
> @@ -5650,8 +5652,9 @@ get_tag_message(char **tagmsg, char **tagmsg_path, con
>  		goto done;
>  	}
>  
> -	if (asprintf(&initial_content, "\n# tagging commit %s as %s\n",
> -	    commit_id_str, tag_name) == -1) {
> +	if ((initial_content_len = asprintf(&initial_content,
> +	    "\n# tagging commit %s as %s\n",
> +	    commit_id_str, tag_name)) == -1) {
>  		err = got_error_from_errno("asprintf");
>  		goto done;
>  	}
> @@ -5660,7 +5663,7 @@ get_tag_message(char **tagmsg, char **tagmsg_path, con
>  	if (err)
>  		goto done;
>  
> -	dprintf(fd, initial_content);
> +	write(fd, initial_content, initial_content_len);
>  	close(fd);
>  
>  	err = get_editor(&editor);
> @@ -6501,6 +6504,7 @@ collect_commit_logmsg(struct got_pathlist_head *commit
>      void *arg)
>  {
>  	char *initial_content = NULL;
> +	size_t initial_content_len;
>  	struct got_pathlist_entry *pe;
>  	const struct got_error *err = NULL;
>  	char *template = NULL;
> @@ -6521,16 +6525,16 @@ collect_commit_logmsg(struct got_pathlist_head *commit
>  	if (asprintf(&template, "%s/logmsg", a->worktree_path) == -1)
>  		return got_error_from_errno("asprintf");
>  
> -	if (asprintf(&initial_content,
> +	if ((initial_content_len = asprintf(&initial_content,
>  	    "\n# changes to be committed on branch %s:\n",
> -	    a->branch_name) == -1)
> +	    a->branch_name)) == -1)
>  		return got_error_from_errno("asprintf");
>  
>  	err = got_opentemp_named_fd(&a->logmsg_path, &fd, template);
>  	if (err)
>  		goto done;
>  
> -	dprintf(fd, initial_content);
> +	write(fd, initial_content, initial_content_len);
>  
>  	TAILQ_FOREACH(pe, commitable_paths, entry) {
>  		struct got_commitable *ct = pe->data;
> @@ -7713,6 +7717,7 @@ histedit_edit_logmsg(struct got_histedit_list_entry *h
>  {
>  	char *logmsg_path = NULL, *id_str = NULL, *orig_logmsg = NULL;
>  	char *logmsg = NULL, *new_msg = NULL, *editor = NULL;
> +	size_t logmsg_len;
>  	const struct got_error *err = NULL;
>  	struct got_commit_object *commit = NULL;
>  	int fd;
> @@ -7745,9 +7750,9 @@ histedit_edit_logmsg(struct got_histedit_list_entry *h
>  	err = got_object_commit_get_logmsg(&orig_logmsg, commit);
>  	if (err)
>  		goto done;
> -	if (asprintf(&new_msg,
> +	if ((logmsg_len = asprintf(&new_msg,
>  	    "%s\n# original log message of commit %s: %s",
> -	    logmsg ? logmsg : "", id_str, orig_logmsg) == -1) {
> +	    logmsg ? logmsg : "", id_str, orig_logmsg)) == -1) {
>  		err = got_error_from_errno("asprintf");
>  		goto done;
>  	}
> @@ -7763,7 +7768,7 @@ histedit_edit_logmsg(struct got_histedit_list_entry *h
>  	if (err)
>  		goto done;
>  
> -	dprintf(fd, logmsg);
> +	write(fd, logmsg, logmsg_len);
>  	close(fd);
>  
>  	err = get_editor(&editor);
> -- 
> Christian "naddy" Weisgerber                          naddy@mips.inka.de

-- 

Tracey Emery