Add some error checking and clean out some cruft.

Error checking #1: When a cached dex file can't be created, do extra
analysis to figure out (and report) why.

Error checking #2: When opening classpath entries, become sensitive
to the file extension, only trying to open files with the right
extensions and complaining explicitly if it's unrecognized.

Cruft cleaning: We've never supported finding class files in directory
hierarchies in Dalvik. Fix some related comments and clean out some
code that tried (in vain) to implement a piece of that.

Bug: 4523201
Change-Id: I05b7a8570f147955cd62229fca72b50d36703752
diff --git a/libdex/OptInvocation.cpp b/libdex/OptInvocation.cpp
index 911e9d5..4e88c24 100644
--- a/libdex/OptInvocation.cpp
+++ b/libdex/OptInvocation.cpp
@@ -13,9 +13,11 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 /*
- * Utility functions for managing an invocation of "dexopt".
+ * Utility functions for dealing with optimized dex files.
  */
+
 #include "vm/DalvikVersion.h"
 
 #include <stdint.h>
@@ -30,9 +32,9 @@
 #include "OptInvocation.h"
 #include "DexFile.h"
 
+static const char* kCacheDirectoryName = "dalvik-cache";
 static const char* kClassesDex = "classes.dex";
 
-
 /*
  * Given the filename of a .jar or .dex file, construct the DEX file cache
  * name.
@@ -45,7 +47,6 @@
 char* dexOptGenerateCacheFileName(const char* fileName, const char* subFileName)
 {
     char nameBuf[512];
-    static const char kDexCachePath[] = "dalvik-cache";
     char absoluteFile[sizeof(nameBuf)];
     const size_t kBufLen = sizeof(nameBuf) - 1;
     const char* dataRoot;
@@ -95,7 +96,7 @@
     dataRoot = getenv("ANDROID_DATA");
     if (dataRoot == NULL)
         dataRoot = "/data";
-    snprintf(nameBuf, kBufLen, "%s/%s", dataRoot, kDexCachePath);
+    snprintf(nameBuf, kBufLen, "%s/%s", dataRoot, kCacheDirectoryName);
 
     /* Tack on the file name for the actual cache file path.
      */
diff --git a/vm/analysis/DexPrepare.cpp b/vm/analysis/DexPrepare.cpp
index 1a817f5..f8fe327 100644
--- a/vm/analysis/DexPrepare.cpp
+++ b/vm/analysis/DexPrepare.cpp
@@ -31,14 +31,18 @@
 
 #include <zlib.h>
 
+#include <libgen.h>
 #include <stdlib.h>
 #include <unistd.h>
 #include <sys/mman.h>
 #include <sys/stat.h>
 #include <sys/file.h>
+#include <sys/stat.h>
+#include <sys/types.h>
 #include <sys/wait.h>
 #include <fcntl.h>
 #include <errno.h>
+#include <unistd.h>
 
 
 /* fwd */
@@ -55,6 +59,68 @@
     const RegisterMapBuilder* pRegMapBuilder);
 static bool computeFileChecksum(int fd, off_t start, size_t length, u4* pSum);
 
+/*
+ * Get just the directory portion of the given path. This is just like
+ * dirname(), except it (a) never modifies its argument and (b) always
+ * returns allocated storage that must subsequently be free()d.
+ */
+static char* saneDirName(const char* fileName) {
+    const char* lastSlash = strrchr(fileName, '/');
+
+    if (lastSlash == NULL) {
+        return strdup("."); // strdup() to make free() always be appropriate.
+    }
+
+    size_t length = lastSlash - fileName + 1; // +1 for the '\0' byte.
+    char* result = (char*) malloc(length);
+
+    if (result != NULL) {
+        strlcpy(result, fileName, length);
+    }
+
+    return result;
+}
+
+/*
+ * Helper for dvmOpenCacheDexFile() in a known-error case: Check to
+ * see if the directory part of the given path (all but the last
+ * component) exists and is writable. Complain to the log if not.
+ */
+static bool directoryIsValid(const char* fileName)
+{
+    char* dirName = saneDirName(fileName);
+
+    if (dirName == NULL) {
+        LOGE("Could not get directory name of dex cache file '%s': %s", fileName, strerror(errno));
+        return false;
+    }
+
+    bool ok = true;
+    struct stat status;
+
+    if (stat(dirName, &status) < 0) {
+        LOGE("Could not stat dex cache directory '%s': %s", dirName, strerror(errno));
+        ok = false;
+    }
+
+    if (ok && !S_ISDIR(status.st_mode)) {
+        LOGE("Dex cache directory isn't a directory: %s", dirName);
+        ok = false;
+    }
+
+    if (ok && access(dirName, R_OK) < 0) {
+        LOGE("Dex cache directory isn't readable: %s", dirName);
+        ok = false;
+    }
+
+    if (ok && access(dirName, W_OK) < 0) {
+        LOGE("Dex cache directory isn't writable: %s", dirName);
+        ok = false;
+    }
+
+    free(dirName);
+    return ok;
+}
 
 /*
  * Return the fd of an open file in the DEX file cache area.  If the cache
@@ -98,8 +164,10 @@
         fd = open(cacheFileName, O_RDONLY, 0);
         if (fd < 0) {
             if (createIfMissing) {
-                LOGE("Can't open dex cache '%s': %s",
-                    cacheFileName, strerror(errno));
+                const char* errnoString = strerror(errno);
+                if (directoryIsValid(cacheFileName)) {
+                    LOGE("Can't open dex cache file '%s': %s", cacheFileName, errnoString);
+                }
             }
             return fd;
         }
diff --git a/vm/oo/Class.cpp b/vm/oo/Class.cpp
index 4c9394c..bc33103 100644
--- a/vm/oo/Class.cpp
+++ b/vm/oo/Class.cpp
@@ -509,7 +509,6 @@
         const char* kindStr;
 
         switch (cpe->kind) {
-        case kCpeDir:       kindStr = "dir";    break;
         case kCpeJar:       kindStr = "jar";    break;
         case kCpeDex:       kindStr = "dex";    break;
         default:            kindStr = "???";    break;
@@ -572,8 +571,7 @@
             dvmRawDexFileFree((RawDexFile*) cpe->ptr);
             break;
         default:
-            /* e.g. kCpeDir */
-            assert(cpe->ptr == NULL);
+            assert(false);
             break;
         }
 
@@ -585,6 +583,18 @@
 }
 
 /*
+ * Get the filename suffix of the given file (everything after the
+ * last "." if any, or "<none>" if there's no apparent suffix). The
+ * passed-in buffer will always be '\0' terminated.
+ */
+static void getFileNameSuffix(const char* fileName, char* suffixBuf, size_t suffixBufLen)
+{
+    const char* lastDot = strrchr(fileName, '.');
+
+    strlcpy(suffixBuf, (lastDot == NULL) ? "<none>" : (lastDot + 1), suffixBufLen);
+}
+
+/*
  * Prepare a ClassPathEntry struct, which at this point only has a valid
  * filename.  We need to figure out what kind of file it is, and for
  * everything other than directories we need to open it up and see
@@ -592,43 +602,37 @@
  */
 static bool prepareCpe(ClassPathEntry* cpe, bool isBootstrap)
 {
-    JarFile* pJarFile = NULL;
-    RawDexFile* pRawDexFile = NULL;
     struct stat sb;
-    int cc;
 
-    cc = stat(cpe->fileName, &sb);
-    if (cc < 0) {
+    if (stat(cpe->fileName, &sb) < 0) {
         LOGD("Unable to stat classpath element '%s'", cpe->fileName);
         return false;
     }
     if (S_ISDIR(sb.st_mode)) {
-        /*
-         * The directory will usually have .class files in subdirectories,
-         * which may be a few levels down.  Doing a recursive scan and
-         * caching the results would help us avoid hitting the filesystem
-         * on misses.  Whether or not this is of measureable benefit
-         * depends on a number of factors, but most likely it is not
-         * worth the effort (especially since most of our stuff will be
-         * in DEX or JAR).
-         */
-        cpe->kind = kCpeDir;
-        assert(cpe->ptr == NULL);
-        return true;
+        LOGE("Directory classpath elements are not supported: %s", cpe->fileName);
+        return false;
     }
 
-    if (dvmJarFileOpen(cpe->fileName, NULL, &pJarFile, isBootstrap) == 0) {
-        cpe->kind = kCpeJar;
-        cpe->ptr = pJarFile;
-        return true;
-    }
+    char suffix[10];
+    getFileNameSuffix(cpe->fileName, suffix, sizeof(suffix));
 
-    // TODO: do we still want to support "raw" DEX files in the classpath?
-    if (dvmRawDexFileOpen(cpe->fileName, NULL, &pRawDexFile, isBootstrap) == 0)
-    {
-        cpe->kind = kCpeDex;
-        cpe->ptr = pRawDexFile;
-        return true;
+    if ((strcmp(suffix, "jar") == 0) || (strcmp(suffix, "zip") == 0) ||
+            (strcmp(suffix, "apk") == 0)) {
+        JarFile* pJarFile = NULL;
+        if (dvmJarFileOpen(cpe->fileName, NULL, &pJarFile, isBootstrap) == 0) {
+            cpe->kind = kCpeJar;
+            cpe->ptr = pJarFile;
+            return true;
+        }
+    } else if (strcmp(suffix, "dex") == 0) {
+        RawDexFile* pRawDexFile = NULL;
+        if (dvmRawDexFileOpen(cpe->fileName, NULL, &pRawDexFile, isBootstrap) == 0) {
+            cpe->kind = kCpeDex;
+            cpe->ptr = pRawDexFile;
+            return true;
+        }
+    } else {
+        LOGE("Unknown type suffix '%s'", suffix);
     }
 
     LOGD("Unable to process classpath element '%s'", cpe->fileName);
@@ -729,10 +733,12 @@
     }
     assert(idx <= count);
     if (idx == 0 && !gDvm.optimizing) {
+        /*
+         * There's no way the vm will be doing anything if this is the
+         * case, so just bail out (reasonably) gracefully.
+         */
         LOGE("No valid entries found in bootclasspath '%s'", pathStr);
-        free(cpe);
-        cpe = NULL;
-        goto bail;
+        dvmAbort();
     }
 
     LOGVV("  (filled %d of %d slots)", idx, count);
@@ -771,10 +777,6 @@
         //LOGV("+++  checking '%s' (%d)", cpe->fileName, cpe->kind);
 
         switch (cpe->kind) {
-        case kCpeDir:
-            LOGW("Directory entries ('%s') not supported in bootclasspath",
-                cpe->fileName);
-            break;
         case kCpeJar:
             {
                 JarFile* pJarFile = (JarFile*) cpe->ptr;
@@ -898,11 +900,6 @@
     char urlBuf[strlen(name) + strlen(cpe->fileName) + kUrlOverhead +1];
 
     switch (cpe->kind) {
-    case kCpeDir:
-        sprintf(urlBuf, "file://%s/%s", cpe->fileName, name);
-        if (access(urlBuf+7, F_OK) != 0)
-            goto bail;
-        break;
     case kCpeJar:
         {
             JarFile* pJarFile = (JarFile*) cpe->ptr;
diff --git a/vm/oo/Class.h b/vm/oo/Class.h
index 025989e..f859a6d 100644
--- a/vm/oo/Class.h
+++ b/vm/oo/Class.h
@@ -26,18 +26,16 @@
  * look for optional packages (a/k/a standard extensions), and then try
  * the classpath.
  *
- * In Dalvik, a class can be found in one of three ways:
- *  - as a "loose" .class file in a directory
- *  - as a .class file held in a JAR archive
+ * In Dalvik, a class can be found in one of two ways:
  *  - in a .dex file
+ *  - in a .dex file named specifically "classes.dex", which is held
+ *    inside a jar file
  *
- * These three may be freely intermixed in a classpath specification.
- * Ordering is significant.  (Currently only ".dex" is supported directly
- * by the VM.)
+ * These two may be freely intermixed in a classpath specification.
+ * Ordering is significant.
  */
 enum ClassPathEntryKind {
     kCpeUnknown = 0,
-    kCpeDir,
     kCpeJar,
     kCpeDex,
     kCpeLastEntry       /* used as sentinel at end of array */