From: Jonathan Nieder <jrnieder@gmail.com>
To: Erik Faye-Lund <kusmabite@gmail.com>
Cc: git@vger.kernel.org, msysgit@googlegroups.com,
Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
Subject: Re: [PATCH 1/2] prefer xwrite instead of write
Date: Fri, 17 Jan 2014 10:21:48 -0800 [thread overview]
Message-ID: <20140117182148.GY18964@google.com> (raw)
In-Reply-To: <1389968230-1224-1-git-send-email-kusmabite@gmail.com>
Hi,
Erik Faye-Lund wrote:
> --- 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? (*)
[...]
> --- 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.
[...]
> --- 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.
next prev parent reply other threads:[~2014-01-17 18:21 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-01-17 14:17 [PATCH 1/2] prefer xwrite instead of write Erik Faye-Lund
2014-01-17 14:17 ` [PATCH 2/2] mingw: remove mingw_write Erik Faye-Lund
2014-01-17 18:21 ` Jonathan Nieder [this message]
2014-01-17 19:07 ` [PATCH 1/2] prefer xwrite instead of write Junio C Hamano
2014-01-17 19:08 ` Erik Faye-Lund
2014-01-17 20:02 ` Jonathan Nieder
2014-01-17 20:07 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140117182148.GY18964@google.com \
--to=jrnieder@gmail.com \
--cc=git@vger.kernel.org \
--cc=ilari.liusvaara@elisanet.fi \
--cc=kusmabite@gmail.com \
--cc=msysgit@googlegroups.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).