Re: [PATCH 1/7] files-backend: prefer "0" for write_in_full() error check

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff King wrote:
> Commit 06f46f237a (avoid "write_in_full(fd, buf, len) !=
> len" pattern, 2017-09-13) converted this callsite from:
>
>   write_in_full(...) != 1
>
> to
>
>   write_in_full(...) < 0
>
> But during the conflict resolution in c50424a6f0 (Merge
> branch 'jk/write-in-full-fix', 2017-09-25), this morphed
> into
>
>   write_in_full(...) < 1
>
> This behaves as we want, but we prefer to avoid modeling the
> "less than length" error-check which can be subtly buggy, as
> shown in efacf609c8 (config: avoid "write_in_full(fd, buf,
> len) < len" pattern, 2017-09-13).
>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
>  refs/files-backend.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Good eyes.  Thanks.
Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx>

By the way, the description from that merge commit

    Many codepaths did not diagnose write failures correctly when disks
    go full, due to their misuse of write_in_full() helper function,
    which have been corrected.

does not look accurate to me.  At least the "Many codepaths" part.
All of those changes except for three should be no-ops.  The scariest
one is the 'long ret = write_in_full(fd, buf, size)' in notes-merge.c,
which is about overflowing a "long" on Windows instead of error
handling.

Thanks,
Jonathan



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux