Merge branch 'jk/write-in-full-fix' into maint
authorJunio C Hamano <gitster@pobox.com>
Mon, 23 Oct 2017 05:37:21 +0000 (14:37 +0900)
committerJunio C Hamano <gitster@pobox.com>
Mon, 23 Oct 2017 05:37:22 +0000 (14:37 +0900)
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.

* jk/write-in-full-fix:
read_pack_header: handle signed/unsigned comparison in read result
config: flip return value of store_write_*()
notes-merge: use ssize_t for write_in_full() return value
pkt-line: check write_in_full() errors against "< 0"
convert less-trivial versions of "write_in_full() != len"
avoid "write_in_full(fd, buf, len) != len" pattern
get-tar-commit-id: check write_in_full() return against 0
config: avoid "write_in_full(fd, buf, len) < len" pattern

22 files changed:
builtin/get-tar-commit-id.c
builtin/receive-pack.c
builtin/rerere.c
builtin/unpack-file.c
config.c
diff.c
entry.c
fast-import.c
http-backend.c
ll-merge.c
notes-merge.c
pkt-line.c
read-cache.c
refs.c
refs/files-backend.c
rerere.c
sha1_file.c
shallow.c
streaming.c
t/helper/test-delta.c
transport-helper.c
wrapper.c
index e21c5416cd478eab3b0dddcba5ca9eeefad83d34..6d9a79f9b39834d953685b2641354915df3cfcbe 100644 (file)
@@ -33,8 +33,7 @@ int cmd_get_tar_commit_id(int argc, const char **argv, const char *prefix)
        if (!skip_prefix(content, "52 comment=", &comment))
                return 1;
 
-       n = write_in_full(1, comment, 41);
-       if (n < 41)
+       if (write_in_full(1, comment, 41) < 0)
                die_errno("git get-tar-commit-id: write error");
 
        return 0;
index cabdc55e0933adb09d459a5a0747c718fd2c7d1e..01dea59f5851158c8dff6a055f5cd43d763e84fc 100644 (file)
@@ -742,7 +742,7 @@ static int run_and_feed_hook(const char *hook_name, feed_fn feed,
                size_t n;
                if (feed(feed_state, &buf, &n))
                        break;
-               if (write_in_full(proc.in, buf, n) != n)
+               if (write_in_full(proc.in, buf, n) < 0)
                        break;
        }
        close(proc.in);
index ffb66e29073c07c43197ae34f2c31d3b9b610903..0bc40298c2417a6d1f25aafb4b3577243e985c57 100644 (file)
@@ -18,7 +18,7 @@ static int outf(void *dummy, mmbuffer_t *ptr, int nbuf)
 {
        int i;
        for (i = 0; i < nbuf; i++)
-               if (write_in_full(1, ptr[i].ptr, ptr[i].size) != ptr[i].size)
+               if (write_in_full(1, ptr[i].ptr, ptr[i].size) < 0)
                        return -1;
        return 0;
 }
index 73f133419167840d150300faa696bc32027cd02f..672a54fcdf62f147b0497c690cb7daed07c7534b 100644 (file)
@@ -15,7 +15,7 @@ static char *create_temp_file(unsigned char *sha1)
 
        xsnprintf(path, sizeof(path), ".merge_file_XXXXXX");
        fd = xmkstemp(path);
-       if (write_in_full(fd, buf, size) != size)
+       if (write_in_full(fd, buf, size) < 0)
                die_errno("unable to write temp-file");
        close(fd);
        return path;
index 1a8e24f32ceda05861fda9704e195a81b28ce8ce..17e13493700e0a6883cc1ba26f00cdfc6900342d 100644 (file)
--- a/config.c
+++ b/config.c
@@ -2247,10 +2247,11 @@ static int write_error(const char *filename)
        return 4;
 }
 
-static int store_write_section(int fd, const char *key)
+static ssize_t write_section(int fd, const char *key)
 {
        const char *dot;
-       int i, success;
+       int i;
+       ssize_t ret;
        struct strbuf sb = STRBUF_INIT;
 
        dot = memchr(key, '.', store.baselen);
@@ -2266,15 +2267,16 @@ static int store_write_section(int fd, const char *key)
                strbuf_addf(&sb, "[%.*s]\n", store.baselen, key);
        }
 
-       success = write_in_full(fd, sb.buf, sb.len) == sb.len;
+       ret = write_in_full(fd, sb.buf, sb.len);
        strbuf_release(&sb);
 
-       return success;
+       return ret;
 }
 
-static int store_write_pair(int fd, const char *key, const char *value)
+static ssize_t write_pair(int fd, const char *key, const char *value)
 {
-       int i, success;
+       int i;
+       ssize_t ret;
        int length = strlen(key + store.baselen + 1);
        const char *quote = "";
        struct strbuf sb = STRBUF_INIT;
@@ -2314,10 +2316,10 @@ static int store_write_pair(int fd, const char *key, const char *value)
                }
        strbuf_addf(&sb, "%s\n", quote);
 
-       success = write_in_full(fd, sb.buf, sb.len) == sb.len;
+       ret = write_in_full(fd, sb.buf, sb.len);
        strbuf_release(&sb);
 
-       return success;
+       return ret;
 }
 
 static ssize_t find_beginning_of_line(const char *contents, size_t size,
@@ -2446,8 +2448,8 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
                }
 
                store.key = (char *)key;
-               if (!store_write_section(fd, key) ||
-                   !store_write_pair(fd, key, value))
+               if (write_section(fd, key) < 0 ||
+                   write_pair(fd, key, value) < 0)
                        goto write_err_out;
        } else {
                struct stat st;
@@ -2557,11 +2559,10 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
                        /* write the first part of the config */
                        if (copy_end > copy_begin) {
                                if (write_in_full(fd, contents + copy_begin,
-                                                 copy_end - copy_begin) <
-                                   copy_end - copy_begin)
+                                                 copy_end - copy_begin) < 0)
                                        goto write_err_out;
                                if (new_line &&
-                                   write_str_in_full(fd, "\n") != 1)
+                                   write_str_in_full(fd, "\n") < 0)
                                        goto write_err_out;
                        }
                        copy_begin = store.offset[i];
@@ -2570,18 +2571,17 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
                /* write the pair (value == NULL means unset) */
                if (value != NULL) {
                        if (store.state == START) {
-                               if (!store_write_section(fd, key))
+                               if (write_section(fd, key) < 0)
                                        goto write_err_out;
                        }
-                       if (!store_write_pair(fd, key, value))
+                       if (write_pair(fd, key, value) < 0)
                                goto write_err_out;
                }
 
                /* write the rest of the config */
                if (copy_begin < contents_sz)
                        if (write_in_full(fd, contents + copy_begin,
-                                         contents_sz - copy_begin) <
-                           contents_sz - copy_begin)
+                                         contents_sz - copy_begin) < 0)
                                goto write_err_out;
 
                munmap(contents, contents_sz);
@@ -2758,7 +2758,7 @@ int git_config_rename_section_in_file(const char *config_filename,
                                        continue;
                                }
                                store.baselen = strlen(new_name);
-                               if (!store_write_section(out_fd, new_name)) {
+                               if (write_section(out_fd, new_name) < 0) {
                                        ret = write_error(get_lock_file_path(lock));
                                        goto out;
                                }
@@ -2784,7 +2784,7 @@ int git_config_rename_section_in_file(const char *config_filename,
                if (remove)
                        continue;
                length = strlen(output);
-               if (write_in_full(out_fd, output, length) != length) {
+               if (write_in_full(out_fd, output, length) < 0) {
                        ret = write_error(get_lock_file_path(lock));
                        goto out;
                }
diff --git a/diff.c b/diff.c
index fa77b00492688c67f0cff4d7f1ac54a1868403d4..8406a8324ee281f55f59afbe1406ec1d91c0ca0c 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -2978,7 +2978,7 @@ static void prep_temp_blob(const char *path, struct diff_tempfile *temp,
                blob = buf.buf;
                size = buf.len;
        }
-       if (write_in_full(fd, blob, size) != size)
+       if (write_in_full(fd, blob, size) < 0)
                die_errno("unable to write temp-file");
        close_tempfile(&temp->tempfile);
        temp->name = get_tempfile_path(&temp->tempfile);
diff --git a/entry.c b/entry.c
index 65458f07a4453fa29731fe2b9c20848d6b192b20..3c1818f1d45d92d2470ca549f9596e347086bbb6 100644 (file)
--- a/entry.c
+++ b/entry.c
@@ -244,7 +244,8 @@ static int write_entry(struct cache_entry *ce,
        char *new;
        struct strbuf buf = STRBUF_INIT;
        unsigned long size;
-       size_t wrote, newsize = 0;
+       ssize_t wrote;
+       size_t newsize = 0;
        struct stat st;
        const struct submodule *sub;
 
@@ -319,7 +320,7 @@ static int write_entry(struct cache_entry *ce,
                        fstat_done = fstat_output(fd, state, &st);
                close(fd);
                free(new);
-               if (wrote != size)
+               if (wrote < 0)
                        return error("unable to write file %s", path);
                break;
        case S_IFGITLINK:
index 365d3191aa1abfb42b4d0da6973612126cc74f3b..32ac5323f6310e40595df23b77698dea1622f9fa 100644 (file)
@@ -2951,7 +2951,7 @@ static void parse_reset_branch(const char *arg)
 
 static void cat_blob_write(const char *buf, unsigned long size)
 {
-       if (write_in_full(cat_blob_fd, buf, size) != size)
+       if (write_in_full(cat_blob_fd, buf, size) < 0)
                die_errno("Write to frontend failed");
 }
 
index 519025d2c3d944afb6fdd6ca1f3b091827a67272..5daff8ccaba3d9986e1d4073d0e142c90c01665d 100644 (file)
@@ -357,7 +357,7 @@ static void inflate_request(const char *prog_name, int out, int buffer_input)
                                die("zlib error inflating request, result %d", ret);
 
                        n = stream.total_out - cnt;
-                       if (write_in_full(out, out_buf, n) != n)
+                       if (write_in_full(out, out_buf, n) < 0)
                                die("%s aborted reading request", prog_name);
                        cnt += n;
 
@@ -378,7 +378,7 @@ static void copy_request(const char *prog_name, int out)
        ssize_t n = read_request(0, &buf);
        if (n < 0)
                die_errno("error reading request body");
-       if (write_in_full(out, buf, n) != n)
+       if (write_in_full(out, buf, n) < 0)
                die("%s aborted reading request", prog_name);
        close(out);
        free(buf);
index 9fb855a90030e07e28c1ae9d0994006da22cd4ed..a6ad2ec12dc9c1ece81f120196c81fd259e40eb6 100644 (file)
@@ -154,7 +154,7 @@ static void create_temp(mmfile_t *src, char *path, size_t len)
 
        xsnprintf(path, len, ".merge_file_XXXXXX");
        fd = xmkstemp(path);
-       if (write_in_full(fd, src->ptr, src->size) != src->size)
+       if (write_in_full(fd, src->ptr, src->size) < 0)
                die_errno("unable to write temp-file");
        close(fd);
 }
index c12b354f1003a29c6faf58e7ec970e4ac761812e..01cecbdda36795f74d727dcfab975b913757c349 100644 (file)
@@ -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)
index f364944b931a756b3b4819fb49ff5e9e01daf676..647bbd3bceda71f15fdf137a37f3fa53e6fa6d86 100644 (file)
@@ -94,9 +94,9 @@ void packet_flush(int fd)
 int packet_flush_gently(int fd)
 {
        packet_trace("0000", 4, 1);
-       if (write_in_full(fd, "0000", 4) == 4)
-               return 0;
-       return error("flush packet write failed");
+       if (write_in_full(fd, "0000", 4) < 0)
+               return error("flush packet write failed");
+       return 0;
 }
 
 void packet_buf_flush(struct strbuf *buf)
@@ -137,19 +137,18 @@ static int packet_write_fmt_1(int fd, int gently,
                              const char *fmt, va_list args)
 {
        static struct strbuf buf = STRBUF_INIT;
-       ssize_t count;
 
        strbuf_reset(&buf);
        format_packet(&buf, fmt, args);
-       count = write_in_full(fd, buf.buf, buf.len);
-       if (count == buf.len)
-               return 0;
-
-       if (!gently) {
-               check_pipe(errno);
-               die_errno("packet write with format failed");
+       if (write_in_full(fd, buf.buf, buf.len) < 0) {
+               if (!gently) {
+                       check_pipe(errno);
+                       die_errno("packet write with format failed");
+               }
+               return error("packet write with format failed");
        }
-       return error("packet write with format failed");
+
+       return 0;
 }
 
 void packet_write_fmt(int fd, const char *fmt, ...)
@@ -184,9 +183,9 @@ static int packet_write_gently(const int fd_out, const char *buf, size_t size)
        packet_size = size + 4;
        set_packet_header(packet_write_buffer, packet_size);
        memcpy(packet_write_buffer + 4, buf, size);
-       if (write_in_full(fd_out, packet_write_buffer, packet_size) == packet_size)
-               return 0;
-       return error("packet write failed");
+       if (write_in_full(fd_out, packet_write_buffer, packet_size) < 0)
+               return error("packet write failed");
+       return 0;
 }
 
 void packet_buf_write(struct strbuf *buf, const char *fmt, ...)
index acfb028f480b5545aa82d5c0533fe8bdfc581dfd..5e6d24d44450d9408b461f05cf55b51035ae11dc 100644 (file)
@@ -1921,7 +1921,7 @@ static int ce_write_flush(git_SHA_CTX *context, int fd)
        unsigned int buffered = write_buffer_len;
        if (buffered) {
                git_SHA1_Update(context, write_buffer, buffered);
-               if (write_in_full(fd, write_buffer, buffered) != buffered)
+               if (write_in_full(fd, write_buffer, buffered) < 0)
                        return -1;
                write_buffer_len = 0;
        }
@@ -1970,7 +1970,7 @@ static int ce_flush(git_SHA_CTX *context, int fd, unsigned char *sha1)
 
        /* Flush first if not enough space for SHA1 signature */
        if (left + 20 > WRITE_BUFFER_SIZE) {
-               if (write_in_full(fd, write_buffer, left) != left)
+               if (write_in_full(fd, write_buffer, left) < 0)
                        return -1;
                left = 0;
        }
@@ -1979,7 +1979,7 @@ static int ce_flush(git_SHA_CTX *context, int fd, unsigned char *sha1)
        git_SHA1_Final(write_buffer + left, context);
        hashcpy(sha1, write_buffer + left);
        left += 20;
-       return (write_in_full(fd, write_buffer, left) != left) ? -1 : 0;
+       return (write_in_full(fd, write_buffer, left) < 0) ? -1 : 0;
 }
 
 static void ce_smudge_racily_clean_entry(struct cache_entry *ce)
diff --git a/refs.c b/refs.c
index 41aec6505d1309fa2efed2bd9aca0a0ef2646444..7edcf6c91f38e3d07b42939e35dbcebb3662261e 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -592,7 +592,7 @@ static int write_pseudoref(const char *pseudoref, const unsigned char *sha1,
                }
        }
 
-       if (write_in_full(fd, buf.buf, buf.len) != buf.len) {
+       if (write_in_full(fd, buf.buf, buf.len) < 0) {
                strbuf_addf(err, "could not write to '%s'", filename);
                rollback_lock_file(&lock);
                goto done;
index 0404f2c2333c0fadeb57d225deedc6f0d4afadad..f21a954ce712bb06aae3b83992e20910dc6b061c 100644 (file)
@@ -2039,7 +2039,7 @@ static int log_ref_write_fd(int fd, const struct object_id *old_oid,
 
        written = len <= maxlen ? write_in_full(fd, logrec, len) : -1;
        free(logrec);
-       if (written != len)
+       if (written < 0)
                return -1;
 
        return 0;
@@ -2118,8 +2118,8 @@ static int write_ref_to_lockfile(struct ref_lock *lock,
                return -1;
        }
        fd = get_lock_file_fd(lock->lk);
-       if (write_in_full(fd, oid_to_hex(oid), GIT_SHA1_HEXSZ) != GIT_SHA1_HEXSZ ||
-           write_in_full(fd, &term, 1) != 1 ||
+       if (write_in_full(fd, oid_to_hex(oid), GIT_SHA1_HEXSZ) < 0 ||
+           write_in_full(fd, &term, 1) < 0 ||
            close_ref(lock) < 0) {
                strbuf_addf(err,
                            "couldn't write '%s'", get_lock_file_path(lock->lk));
@@ -3338,8 +3338,8 @@ static int files_reflog_expire(struct ref_store *ref_store,
                                        strerror(errno));
                } else if (update &&
                           (write_in_full(get_lock_file_fd(lock->lk),
-                               oid_to_hex(&cb.last_kept_oid), GIT_SHA1_HEXSZ) != GIT_SHA1_HEXSZ ||
-                           write_str_in_full(get_lock_file_fd(lock->lk), "\n") != 1 ||
+                               oid_to_hex(&cb.last_kept_oid), GIT_SHA1_HEXSZ) < 0 ||
+                           write_str_in_full(get_lock_file_fd(lock->lk), "\n") < 0 ||
                            close_ref(lock) < 0)) {
                        status |= error("couldn't write %s",
                                        get_lock_file_path(lock->lk));
index 70634d456cc0b70cdbac1579b82016761252c7d9..51376cf6da3b8ab8f8e0c4f20c307fef9dec8392 100644 (file)
--- a/rerere.c
+++ b/rerere.c
@@ -258,7 +258,7 @@ static int write_rr(struct string_list *rr, int out_fd)
                                    rerere_id_hex(id),
                                    rr->items[i].string, 0);
 
-               if (write_in_full(out_fd, buf.buf, buf.len) != buf.len)
+               if (write_in_full(out_fd, buf.buf, buf.len) < 0)
                        die("unable to write rerere record");
 
                strbuf_release(&buf);
index 5911364a81fa3d1351fa13d14a63c8854fd60e22..4d0e0c554d348d4104157bc16dd4795c1810689a 100644 (file)
@@ -3715,7 +3715,7 @@ int index_path(unsigned char *sha1, const char *path, struct stat *st, unsigned
 
 int read_pack_header(int fd, struct pack_header *header)
 {
-       if (read_in_full(fd, header, sizeof(*header)) < sizeof(*header))
+       if (read_in_full(fd, header, sizeof(*header)) != sizeof(*header))
                /* "eof before pack header was fully read" */
                return PH_ERROR_EOF;
 
index 54359d549075b5cfe1670ca979ff3cf5eeedc9d1..e8429a9a845f98f4a73299c044463ce99ebbea93 100644 (file)
--- a/shallow.c
+++ b/shallow.c
@@ -296,7 +296,7 @@ const char *setup_temporary_shallow(const struct oid_array *extra)
        if (write_shallow_commits(&sb, 0, extra)) {
                fd = xmks_tempfile(&temporary_shallow, git_path("shallow_XXXXXX"));
 
-               if (write_in_full(fd, sb.buf, sb.len) != sb.len)
+               if (write_in_full(fd, sb.buf, sb.len) < 0)
                        die_errno("failed to write to %s",
                                  get_tempfile_path(&temporary_shallow));
                close_tempfile(&temporary_shallow);
@@ -321,7 +321,7 @@ void setup_alternate_shallow(struct lock_file *shallow_lock,
                                       LOCK_DIE_ON_ERROR);
        check_shallow_file_for_update();
        if (write_shallow_commits(&sb, 0, extra)) {
-               if (write_in_full(fd, sb.buf, sb.len) != sb.len)
+               if (write_in_full(fd, sb.buf, sb.len) < 0)
                        die_errno("failed to write to %s",
                                  get_lock_file_path(shallow_lock));
                *alternate_shallow_file = get_lock_file_path(shallow_lock);
@@ -368,7 +368,7 @@ void prune_shallow(int show_only)
                                       LOCK_DIE_ON_ERROR);
        check_shallow_file_for_update();
        if (write_shallow_commits_1(&sb, 0, NULL, SEEN_ONLY)) {
-               if (write_in_full(fd, sb.buf, sb.len) != sb.len)
+               if (write_in_full(fd, sb.buf, sb.len) < 0)
                        die_errno("failed to write to %s",
                                  get_lock_file_path(&shallow_lock));
                commit_lock_file(&shallow_lock);
index 9afa66b8be6c3b5481e4542c033bfebdb2abbfda..c8b85e4498eaedee4142cbe543174ae639cae8e8 100644 (file)
@@ -539,7 +539,7 @@ int stream_blob_to_fd(int fd, const struct object_id *oid, struct stream_filter
                        kept = 0;
                wrote = write_in_full(fd, buf, readlen);
 
-               if (wrote != readlen)
+               if (wrote < 0)
                        goto close_and_exit;
        }
        if (kept && (lseek(fd, kept - 1, SEEK_CUR) == (off_t) -1 ||
index 59937dc1be1c4f0b3d80e3ef3a86e09bff3703b6..591730adc4f3940940fdb4691da0afb81648e353 100644 (file)
@@ -69,7 +69,7 @@ int cmd_main(int argc, const char **argv)
        }
 
        fd = open (argv[4], O_WRONLY|O_CREAT|O_TRUNC, 0666);
-       if (fd < 0 || write_in_full(fd, out_buf, out_size) != out_size) {
+       if (fd < 0 || write_in_full(fd, out_buf, out_size) < 0) {
                perror(argv[4]);
                return 1;
        }
index 2b830b22900545209fd4e697d88c6faf3d1fd431..a72ed18efbfeedffb217a3d66daa9f2626cf6479 100644 (file)
@@ -44,8 +44,7 @@ static void sendline(struct helper_data *helper, struct strbuf *buffer)
 {
        if (debug)
                fprintf(stderr, "Debug: Remote helper: -> %s", buffer->buf);
-       if (write_in_full(helper->helper->in, buffer->buf, buffer->len)
-               != buffer->len)
+       if (write_in_full(helper->helper->in, buffer->buf, buffer->len) < 0)
                die_errno("Full write to remote helper failed");
 }
 
@@ -74,7 +73,7 @@ static void write_constant(int fd, const char *str)
 {
        if (debug)
                fprintf(stderr, "Debug: Remote helper: -> %s", str);
-       if (write_in_full(fd, str, strlen(str)) != strlen(str))
+       if (write_in_full(fd, str, strlen(str)) < 0)
                die_errno("Full write to remote helper failed");
 }
 
index 36630e5d1855a41407e217e610aa1f293288abe1..61aba0b5c1bece4aad04bee649accae990a8ce18 100644 (file)
--- a/wrapper.c
+++ b/wrapper.c
@@ -652,7 +652,7 @@ int xsnprintf(char *dst, size_t max, const char *fmt, ...)
 void write_file_buf(const char *path, const char *buf, size_t len)
 {
        int fd = xopen(path, O_WRONLY | O_CREAT | O_TRUNC, 0666);
-       if (write_in_full(fd, buf, len) != len)
+       if (write_in_full(fd, buf, len) < 0)
                die_errno(_("could not write to %s"), path);
        if (close(fd))
                die_errno(_("could not close %s"), path);