git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 1/2] prefer xwrite instead of write

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Jan 17, 2014, 18:21 UTC
Message-ID
<20140117182148.GY18964@google.com>
In-Reply-To
<1389968230-1224-1-git-send-email-kusmabite@gmail.com>
Hi,
Erik Faye-Lund wrote:
Show 9 quoted lines
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -367,7 +367,7 @@ static void squash_message(struct commit *commit, struct commit_list *remotehead
>  			sha1_to_hex(commit->object.sha1));
>  		pretty_print_commit(&ctx, commit, &out);
>  	}
> -	if (write(fd, out.buf, out.len) < 0)
> +	if (xwrite(fd, out.buf, out.len) < 0)
>  		die_errno(_("Writing SQUASH_MSG"));
Shouldn't this use write_in_full() to avoid a silently truncated result? (*)
[...]
Show 8 quoted lines
> --- a/streaming.c
> +++ b/streaming.c
> @@ -538,7 +538,7 @@ int stream_blob_to_fd(int fd, unsigned const char *sha1, struct stream_filter *f
>  			goto close_and_exit;
>  	}
>  	if (kept && (lseek(fd, kept - 1, SEEK_CUR) == (off_t) -1 ||
> -		     write(fd, "", 1) != 1))
> +		     xwrite(fd, "", 1) != 1))
Yeah, if we get EINTR then it's worth retrying.
[...]
Show 11 quoted lines
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -1129,9 +1129,8 @@ static int udt_do_write(struct unidirectional_transfer *t)
>  		return 0;	/* Nothing to write. */
>  
>  	transfer_debug("%s is writable", t->dest_name);
> -	bytes = write(t->dest, t->buf, t->bufuse);
> -	if (bytes < 0 && errno != EWOULDBLOCK && errno != EAGAIN &&
> -		errno != EINTR) {
> +	bytes = xwrite(t->dest, t->buf, t->bufuse);
> +	if (bytes < 0 && errno != EWOULDBLOCK) {

Here the write is limited by BUFFERSIZE, and returning to the outer loop to try another read when the write returns EAGAIN, like the original code does, seems philosophically like the right thing to do.

Luckily we don't use O_NONBLOCK anywhere, so the change shouldn't matter in practice. So although it doesn't do any good, using xwrite here for consistency should be fine.

So my only worry is the (*) above.  With that change,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
Previous: Erik Faye-LundNext: Junio C Hamano
Message 3 of 7 in “prefer xwrite instead of write”
  1. 1/2 prefer xwrite instead of writeErik Faye-Lund, Jan 17, 2014
  2. 2/2 mingw: remove mingw_writeErik Faye-Lund, Jan 17, 2014
  3. Jonathan NiederJan 17, 2014
  4. Junio C HamanoJan 17, 2014
  5. Erik Faye-LundJan 17, 2014
  6. Jonathan NiederJan 17, 2014
  7. Junio C HamanoJan 17, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.