Re: [PATCH 6/7] notes-merge: use ssize_t for write_in_full() return value
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Sep 13, 2017, 21:20 UTC
- Message-ID
- <20170913212035.GN27425@aiede.mtv.corp.google.com>
- In-Reply-To
- <20170913171744.32cmtjkdcp7yf7pt@sigill.intra.peff.net>
Jeff King wrote:
Show 12 quoted lines
> We store the return value of write_in_full() in a long, > though the return is actually an ssize_t. This probably > doesn't matter much in practice (since the buffer size is > alredy an unsigned long), but it might if the size if > between what can be represented in "long" and "unsigned > long", and if your size_t is larger than a "long" (as it is > on 64-bit Windows). > > Signed-off-by: Jeff King <peff@peff.net> > --- > notes-merge.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-)
Yikes. Good catch.
With or without the tweak below, Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Show 16 quoted lines
> --- a/notes-merge.c
> +++ b/notes-merge.c
> @@ -302,7 +302,7 @@ static void write_buf_to_worktree(const struct object_id *obj,
> fd = xopen(path, O_WRONLY | O_EXCL | O_CREAT, 0666);
>
> while (size > 0) {
> - long ret = write_in_full(fd, buf, size);
> + ssize_t ret = write_in_full(fd, buf, size);
> if (ret < 0) {
> /* Ignore epipe */
> if (errno == EPIPE)
> break;
> die_errno("notes-merge");
> } else if (!ret) {
> die("notes-merge: disk full?");
> }These three lines are dead code. How about the following, e.g. for squashing in?
diff --git i/notes-merge.c w/notes-merge.c index 597d43f65c..4352c34a6e 100644 --- i/notes-merge.c +++ w/notes-merge.c @@ -308,8 +308,6 @@ static void write_buf_to_worktree(const struct object_id *obj, if (errno == EPIPE) break; die_errno("notes-merge"); - } else if (!ret) { - die("notes-merge: disk full?"); } size -= ret; buf += ret;