Re: [PATCH v3] refs.c: add a function to append a reflog entry to a fd

[Junio C Hamano <gitster@pobox.com>]

Stefan Beller <sbeller@google.com> writes:

> From: Ronnie Sahlberg <sahlberg@google.com>
>
> Move code to create the string for a ref and write it to a file descriptor
> from log_ref_write and add it into a new dedicated function
> log_ref_write_fd.
>
> For now the new function is only used from log_ref_write, but later
> on we will call this function from reflog transactions too. That means
> that we will end up with only a single place, where we write a
> reflog entry to a file instead of the current two places
> (log_ref_write and builtin/reflog.c).
>
> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>
> Changes in version 3:
> * reword the commit message to make it more understandable.
> * no changes in code
> * wait for the follow up to address any changes in the code.
>
>  refs.c | 48 ++++++++++++++++++++++++++++++------------------
>  1 file changed, 30 insertions(+), 18 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 5ff457e..9948841 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2990,15 +2990,37 @@ int log_ref_setup(const char *refname, char *logfile, int bufsize)
>  	return 0;
>  }
>  
> +static int log_ref_write_fd(int fd, const unsigned char *old_sha1,
> +			    const unsigned char *new_sha1,
> +			    const char *committer, const char *msg)
> +{
> +	int msglen, written;
> +	unsigned maxlen, len;
> +	char *logrec;
> +
> +	msglen = msg ? strlen(msg) : 0;
> +	maxlen = strlen(committer) + msglen + 100;
> +	logrec = xmalloc(maxlen);
> +	len = sprintf(logrec, maxlen, "%s %s %s\n",

You cannot have maxlen here ;-)

> +		      sha1_to_hex(old_sha1),
> +		      sha1_to_hex(new_sha1),
> +		      committer);
> +	if (msglen)
> +		len += copy_msg(logrec + len - 1, msg) - 1;
> +
> +	written = len <= maxlen ? write_in_full(fd, logrec, len) : -1;
> +	free(logrec);
> +	if (written != len)
> +		return -1;
> +
> +	return 0;
> +}
> +
>  static int log_ref_write(const char *refname, const unsigned char *old_sha1,
>  			 const unsigned char *new_sha1, const char *msg)
>  {
> -	int logfd, result, written, oflags = O_APPEND | O_WRONLY;
> -	unsigned maxlen, len;
> -	int msglen;
> +	int logfd, result, oflags = O_APPEND | O_WRONLY;
>  	char log_file[PATH_MAX];
> -	char *logrec;
> -	const char *committer;
>  
>  	if (log_all_ref_updates < 0)
>  		log_all_ref_updates = !is_bare_repository();
> @@ -3010,19 +3032,9 @@ static int log_ref_write(const char *refname, const unsigned char *old_sha1,
>  	logfd = open(log_file, oflags);
>  	if (logfd < 0)
>  		return 0;
> -	msglen = msg ? strlen(msg) : 0;
> -	committer = git_committer_info(0);
> -	maxlen = strlen(committer) + msglen + 100;
> -	logrec = xmalloc(maxlen);
> -	len = sprintf(logrec, "%s %s %s\n",
> -		      sha1_to_hex(old_sha1),
> -		      sha1_to_hex(new_sha1),
> -		      committer);
> -	if (msglen)
> -		len += copy_msg(logrec + len - 1, msg) - 1;
> -	written = len <= maxlen ? write_in_full(logfd, logrec, len) : -1;
> -	free(logrec);
> -	if (written != len) {
> +	result = log_ref_write_fd(logfd, old_sha1, new_sha1,
> +				  git_committer_info(0), msg);
> +	if (result) {
>  		int save_errno = errno;
>  		close(logfd);
>  		error("Unable to append to %s", log_file);