From ea5518f69edc51e8e70c2c4d4c4daa3ad9bcb242 Mon Sep 17 00:00:00 2001
From: Ben Noordhuis <info@bnoordhuis.nl>
Date: Sat, 13 Apr 2019 12:31:45 +0200
Subject: make os_file_close poison file handle after close

This helps track down use-after-close bugs.
---
 src/cache_hash.cpp | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

(limited to 'src/cache_hash.cpp')

diff --git a/src/cache_hash.cpp b/src/cache_hash.cpp
index 1f25a9982e..250733ec92 100644
--- a/src/cache_hash.cpp
+++ b/src/cache_hash.cpp
@@ -256,10 +256,10 @@ static Error populate_file_hash(CacheHash *ch, CacheHashFile *chf, Buf *contents
     }
 
     if ((err = hash_file(chf->bin_digest, this_file, contents))) {
-        os_file_close(this_file);
+        os_file_close(&this_file);
         return err;
     }
-    os_file_close(this_file);
+    os_file_close(&this_file);
 
     blake2b_update(&ch->blake, chf->bin_digest, 48);
 
@@ -300,7 +300,7 @@ Error cache_hit(CacheHash *ch, Buf *out_digest) {
     Buf line_buf = BUF_INIT;
     buf_resize(&line_buf, 512);
     if ((err = os_file_read_all(ch->manifest_file, &line_buf))) {
-        os_file_close(ch->manifest_file);
+        os_file_close(&ch->manifest_file);
         return err;
     }
 
@@ -389,14 +389,14 @@ Error cache_hit(CacheHash *ch, Buf *out_digest) {
         OsFileAttr actual_attr;
         if ((err = os_file_open_r(chf->path, &this_file, &actual_attr))) {
             fprintf(stderr, "Unable to open %s\n: %s", buf_ptr(chf->path), err_str(err));
-            os_file_close(ch->manifest_file);
+            os_file_close(&ch->manifest_file);
             return ErrorCacheUnavailable;
         }
         if (chf->attr.mtime.sec == actual_attr.mtime.sec &&
             chf->attr.mtime.nsec == actual_attr.mtime.nsec &&
             chf->attr.inode == actual_attr.inode)
         {
-            os_file_close(this_file);
+            os_file_close(&this_file);
         } else {
             // we have to recompute the digest.
             // later we'll rewrite the manifest with the new mtime/digest values
@@ -411,11 +411,11 @@ Error cache_hit(CacheHash *ch, Buf *out_digest) {
 
             uint8_t actual_digest[48];
             if ((err = hash_file(actual_digest, this_file, nullptr))) {
-                os_file_close(this_file);
-                os_file_close(ch->manifest_file);
+                os_file_close(&this_file);
+                os_file_close(&ch->manifest_file);
                 return err;
             }
-            os_file_close(this_file);
+            os_file_close(&this_file);
             if (memcmp(chf->bin_digest, actual_digest, 48) != 0) {
                 memcpy(chf->bin_digest, actual_digest, 48);
                 // keep going until we have the input file digests
@@ -433,12 +433,12 @@ Error cache_hit(CacheHash *ch, Buf *out_digest) {
             CacheHashFile *chf = &ch->files.at(file_i);
             if ((err = populate_file_hash(ch, chf, nullptr))) {
                 fprintf(stderr, "Unable to hash %s: %s\n", buf_ptr(chf->path), err_str(err));
-                os_file_close(ch->manifest_file);
+                os_file_close(&ch->manifest_file);
                 return ErrorCacheUnavailable;
             }
         }
         if (return_code != ErrorNone) {
-            os_file_close(ch->manifest_file);
+            os_file_close(&ch->manifest_file);
         }
         return return_code;
     }
@@ -453,7 +453,7 @@ Error cache_add_file_fetch(CacheHash *ch, Buf *resolved_path, Buf *contents) {
     CacheHashFile *chf = ch->files.add_one();
     chf->path = resolved_path;
     if ((err = populate_file_hash(ch, chf, contents))) {
-        os_file_close(ch->manifest_file);
+        os_file_close(&ch->manifest_file);
         return err;
     }
 
@@ -586,6 +586,6 @@ void cache_release(CacheHash *ch) {
         }
     }
 
-    os_file_close(ch->manifest_file);
+    os_file_close(&ch->manifest_file);
 }
 
-- 
cgit v1.2.3


From 93e89b3b7efeaa41f4f11fbb022b962d2244dab2 Mon Sep 17 00:00:00 2001
From: Ben Noordhuis <info@bnoordhuis.nl>
Date: Sat, 13 Apr 2019 12:33:29 +0200
Subject: don't close cache manifest file prematurely

ErrorInvalidFormat is not a fatal error so don't close the cache
manifest file right away but instead let cache_final() handle it.

Fixes the following (very common) warning when running the test suite:

    Warning: Unable to write cache file [..]: unexpected seek failure

The seek failure is an lseek() system call that failed with EBADF
because the file descriptor had already been closed.
---
 src/cache_hash.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'src/cache_hash.cpp')

diff --git a/src/cache_hash.cpp b/src/cache_hash.cpp
index 250733ec92..efb1a06b59 100644
--- a/src/cache_hash.cpp
+++ b/src/cache_hash.cpp
@@ -437,7 +437,7 @@ Error cache_hit(CacheHash *ch, Buf *out_digest) {
                 return ErrorCacheUnavailable;
             }
         }
-        if (return_code != ErrorNone) {
+        if (return_code != ErrorNone && return_code != ErrorInvalidFormat) {
             os_file_close(&ch->manifest_file);
         }
         return return_code;
-- 
cgit v1.2.3