Re: [PATCH 6/7] notes-merge: use ssize_t for write_in_full() return value

[Jeff King]

On Wed, Sep 13, 2017 at 02:20:35PM -0700, Jonathan Nieder wrote:

> > --- 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?

Thanks, I didn't notice that.

I'd actually prefer it as a separate patch, since it needs explained
separately.

-Peff

Re: [PATCH 6/7] notes-merge: use ssize_t for write_in_full() return value

[Jonathan Nieder]

Jeff King wrote:

> 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 
> ---
>  notes-merge.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Yikes.  Good catch.

With or without the tweak below,
Reviewed-by: Jonathan Nieder 

> --- 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;