Minor zip cleanups

Change-Id: Iaa2552245e9dac5fabc6a9fb553174e4d5d84142
diff --git a/src/dex_file.cc b/src/dex_file.cc
index 90893cb..b3bb7d2 100644
--- a/src/dex_file.cc
+++ b/src/dex_file.cc
@@ -86,7 +86,7 @@
    static LockedFd* CreateAndLock(std::string& name, mode_t mode) {
     int fd = open(name.c_str(), O_CREAT | O_RDWR, mode);
     if (fd == -1) {
-      PLOG(ERROR) << "Can't open file '" << name;
+      PLOG(ERROR) << "Can't open file '" << name << "'";
       return NULL;
     }
     fchmod(fd, mode);
@@ -98,8 +98,8 @@
         result = flock(fd, LOCK_EX);
     }
     if (result == -1 ) {
+      PLOG(ERROR) << "Can't lock file '" << name << "'";
       close(fd);
-      PLOG(ERROR) << "Can't lock file '" << name;
       return NULL;
     }
     return new LockedFd(fd);
@@ -255,6 +255,10 @@
     uint32_t computed_crc = crc32(0L, Z_NULL, 0);
     while (true) {
       ssize_t bytes_read = TEMP_FAILURE_RETRY(read(fd->GetFd(), buf.get(), kBufSize));
+      if (bytes_read == -1) {
+        PLOG(ERROR) << "Problem computing CRC of '" << cache_path_tmp << "'";
+        return NULL;
+      }
       if (bytes_read == 0) {
         break;
       }
@@ -265,7 +269,8 @@
     }
     int rename_result = rename(cache_path_tmp.c_str(), cache_path.c_str());
     if (rename_result == -1) {
-      PLOG(ERROR) << "Can't install dex cache file '" << cache_path << "' from '" << cache_path_tmp;
+      PLOG(ERROR) << "Can't install dex cache file '" << cache_path << "'"
+                  << " from '" << cache_path_tmp << "'";
       unlink(cache_path.c_str());
     }
   }
diff --git a/src/globals.h b/src/globals.h
index 2826bbd..b7096e6 100644
--- a/src/globals.h
+++ b/src/globals.h
@@ -32,6 +32,14 @@
 const int kBitsPerWord = kWordSize * kBitsPerByte;
 const int kBitsPerInt = kIntSize * kBitsPerByte;
 
+
+// System page size.  Normally you're expected to get this from
+// sysconf(_SC_PAGESIZE) or some system-specific define (usually
+// PAGESIZE or PAGE_SIZE).  If we use a simple compile-time constant
+// the compiler can generate appropriate masks directly, so we define
+// it here and verify it as the runtime is starting up.
+//
+// Must be a power of 2.
 const int kPageSize = 4096;
 
 }  // namespace art
diff --git a/src/zip_archive.cc b/src/zip_archive.cc
index a615321..0303e6f 100644
--- a/src/zip_archive.cc
+++ b/src/zip_archive.cc
@@ -63,30 +63,30 @@
   // off the end of the mapped region.
 
   off_t dir_offset = zip_archive_->dir_offset_;
-  int64_t local_hdr_offset = Le32ToHost(ptr_ + ZipArchive::kCDELocalOffset);
-  if (local_hdr_offset + ZipArchive::kLFHLen >= dir_offset) {
-    LOG(WARNING) << "Zip: bad local hdr offset in zip";
+  int64_t lfh_offset = Le32ToHost(ptr_ + ZipArchive::kCDELocalOffset);
+  if (lfh_offset + ZipArchive::kLFHLen >= dir_offset) {
+    LOG(WARNING) << "Zip: bad LFH offset in zip";
     return -1;
   }
 
-  if (lseek(zip_archive_->fd_, local_hdr_offset, SEEK_SET) != local_hdr_offset) {
-    PLOG(WARNING) << "Zip: failed seeking to lfh at offset " << local_hdr_offset;
+  if (lseek(zip_archive_->fd_, lfh_offset, SEEK_SET) != lfh_offset) {
+    PLOG(WARNING) << "Zip: failed seeking to LFH at offset " << lfh_offset;
     return -1;
   }
 
   uint8_t lfh_buf[ZipArchive::kLFHLen];
   ssize_t actual = TEMP_FAILURE_RETRY(read(zip_archive_->fd_, lfh_buf, sizeof(lfh_buf)));
   if (actual != sizeof(lfh_buf)) {
-    LOG(WARNING) << "Zip: failed reading lfh from offset " << local_hdr_offset;
+    LOG(WARNING) << "Zip: failed reading LFH from offset " << lfh_offset;
     return -1;
   }
 
   if (Le32ToHost(lfh_buf) != ZipArchive::kLFHSignature) {
-    LOG(WARNING) << "Zip: didn't find signature at start of lfh, offset " << local_hdr_offset;
+    LOG(WARNING) << "Zip: didn't find signature at start of LFH, offset " << lfh_offset;
     return -1;
   }
 
-  off_t data_offset = (local_hdr_offset + ZipArchive::kLFHLen
+  off_t data_offset = (lfh_offset + ZipArchive::kLFHLen
                        + Le16ToHost(lfh_buf + ZipArchive::kLFHNameLen)
                        + Le16ToHost(lfh_buf + ZipArchive::kLFHExtraLen));
   if (data_offset >= dir_offset) {
@@ -270,27 +270,12 @@
   }
 }
 
-static bool CloseOnExec(int fd) {
-  int flags = fcntl(fd, F_GETFD);
-  if (flags < 0) {
-    return false;
-  }
-  if (fcntl(fd, F_SETFD, flags | FD_CLOEXEC) < 0) {
-    return false;
-  }
-  return true;
-}
-
 // return new ZipArchive instance on success, NULL on error.
 ZipArchive* ZipArchive::Open(const std::string& filename) {
   DCHECK(!filename.empty());
-  int fd = open(filename.c_str(), O_RDONLY, 0);
+  int fd = open(filename.c_str(), O_RDONLY | O_CLOEXEC, 0);
   if (fd < 0) {
-    int err = errno ? errno : -1;
-    LOG(WARNING) << "Unable to open '" << filename << "': " << strerror(err);
-    return NULL;
-  }
-  if (!CloseOnExec(fd)) {
+    PLOG(WARNING) << "Unable to open '" << filename << "'";
     return NULL;
   }
   scoped_ptr<ZipArchive> zip_archive(new ZipArchive(fd));