From 906e685ce43092812bf403016057376d0657a671 Mon Sep 17 00:00:00 2001
From: Elliott Hughes <enh@google.com>
Date: Fri, 28 Oct 2011 14:52:10 -0700
Subject: Fix the unintelligible IsValidClassName API by breaking it into
 three.

There are only three of the four possibilities you actually want, so we may as
well just call them by name.

Change-Id: I481d91d98d6d47430f1972891fa81a61c57ad331
---
 src/check_jni.cc       |  4 ++--
 src/java_lang_Class.cc |  2 +-
 src/utils.cc           | 29 ++++++++++++++++++-----------
 src/utils.h            | 12 ++++--------
 4 files changed, 25 insertions(+), 22 deletions(-)

(limited to 'src')

diff --git a/src/check_jni.cc b/src/check_jni.cc
index 15cc161..b6670e4 100644
--- a/src/check_jni.cc
+++ b/src/check_jni.cc
@@ -161,7 +161,7 @@ public:
    * "[Ljava/lang/Object;".
    */
   void CheckClassName(const char* className) {
-    if (!IsValidClassName(className, true, false)) {
+    if (!IsValidJniClassName(className)) {
       LOG(ERROR) << "JNI ERROR: illegal class name '" << className << "' (" << function_name_ << ")\n"
                  << "           (should be of the form 'java/lang/String', [Ljava/lang/String;' or '[[B')\n";
       JniAbort();
@@ -241,7 +241,7 @@ public:
     DCHECK(f->GetType() != NULL);
     Class* c = o->GetClass();
     if (c->FindInstanceField(f->GetName()->ToModifiedUtf8(), f->GetTypeDescriptor()) == NULL) {
-      LOG(ERROR) << "JNI ERROR: jfieldID " << PrettyField(f) 
+      LOG(ERROR) << "JNI ERROR: jfieldID " << PrettyField(f)
                  << " not valid for an object of class " << PrettyTypeOf(o);
       JniAbort();
     }
diff --git a/src/java_lang_Class.cc b/src/java_lang_Class.cc
index eccf977..1700ce1 100644
--- a/src/java_lang_Class.cc
+++ b/src/java_lang_Class.cc
@@ -38,7 +38,7 @@ jclass Class_classForName(JNIEnv* env, jclass, jstring javaName, jboolean initia
   // We need to validate and convert the name (from x.y.z to x/y/z).  This
   // is especially handy for array types, since we want to avoid
   // auto-generating bogus array classes.
-  if (!IsValidClassName(name.c_str(), true, true)) {
+  if (!IsValidBinaryClassName(name.c_str())) {
     Thread::Current()->ThrowNewExceptionF("Ljava/lang/ClassNotFoundException;",
         "Invalid name: %s", name.c_str());
     return NULL;
diff --git a/src/utils.cc b/src/utils.cc
index 4c65036..c37343c 100644
--- a/src/utils.cc
+++ b/src/utils.cc
@@ -274,8 +274,6 @@ std::string JniLongName(const Method* m) {
   return long_name;
 }
 
-namespace {
-
 // Helper for IsValidMemberNameUtf8(), a bit vector indicating valid low ascii.
 uint32_t DEX_MEMBER_VALID_LOW_ASCII[4] = {
   0x00000000, // 00..1f low control characters; nothing valid
@@ -355,11 +353,8 @@ bool IsValidMemberNameUtf8(const char** pUtf8Ptr) {
   return IsValidMemberNameUtf8Slow(pUtf8Ptr);
 }
 
-}  // namespace
-
-bool IsValidClassName(const char* s, bool isClassName, bool dot_or_slash) {
-  char separator = (dot_or_slash ? '.' : '/');
-
+enum ClassNameType { kName, kDescriptor };
+bool IsValidClassName(const char* s, ClassNameType type, char separator) {
   int arrayCount = 0;
   while (*s == '[') {
     arrayCount++;
@@ -378,10 +373,10 @@ bool IsValidClassName(const char* s, bool isClassName, bool dot_or_slash) {
      * format looks the same as a type descriptor in that case, so
      * treat it as such.
      */
-    isClassName = false;
+    type = kDescriptor;
   }
 
-  if (!isClassName) {
+  if (type == kDescriptor) {
     /*
      * We are looking for a descriptor. Either validate it as a
      * single-character primitive type, or continue on to check the
@@ -427,7 +422,7 @@ bool IsValidClassName(const char* s, bool isClassName, bool dot_or_slash) {
        * empty component (including the degenerate case of
        * the empty string "").
        */
-      return isClassName && !sepOrFirst;
+      return (type == kName) && !sepOrFirst;
     case ';':
       /*
        * Invalid character for a class name, but the
@@ -436,7 +431,7 @@ bool IsValidClassName(const char* s, bool isClassName, bool dot_or_slash) {
        * and that it doesn't end with an empty component
        * (including the degenerate case of "L;").
        */
-      return !isClassName && !sepOrFirst && (s[1] == '\0');
+      return (type == kDescriptor) && !sepOrFirst && (s[1] == '\0');
     case '/':
     case '.':
       if (c != separator) {
@@ -460,6 +455,18 @@ bool IsValidClassName(const char* s, bool isClassName, bool dot_or_slash) {
   }
 }
 
+bool IsValidBinaryClassName(const char* s) {
+  return IsValidClassName(s, kName, '.');
+}
+
+bool IsValidJniClassName(const char* s) {
+  return IsValidClassName(s, kName, '/');
+}
+
+bool IsValidDescriptor(const char* s) {
+  return IsValidClassName(s, kDescriptor, '/');
+}
+
 void Split(const std::string& s, char delim, std::vector<std::string>& result) {
   const char* p = s.data();
   const char* end = p + s.size();
diff --git a/src/utils.h b/src/utils.h
index bb307d1..f747ae8 100644
--- a/src/utils.h
+++ b/src/utils.h
@@ -190,14 +190,10 @@ std::string DotToDescriptor(const char* class_name);
 // Turn "Ljava/lang/String;" into "java.lang.String".
 std::string DescriptorToDot(const std::string& descriptor);
 
-// Tests whether 's' is a valid class name.
-// name_or_descriptor
-//     true  => "java/lang/String"
-//     false => "Ljava/lang/String;" (i.e. "descriptor")
-// dot_or_slash
-//     true  => "java.lang.String"
-//     false => "java/lang/String" (i.e. "dot or slash")
-bool IsValidClassName(const char* s, bool name_or_descriptor, bool dot_or_slash);
+// Tests for whether 's' is a valid class name in the three common forms:
+bool IsValidBinaryClassName(const char* s);  // "java.lang.String"
+bool IsValidJniClassName(const char* s);     // "java/lang/String"
+bool IsValidDescriptor(const char* s);       // "Ljava/lang/String;"
 
 // Returns the JNI native function name for the non-overloaded method 'm'.
 std::string JniShortName(const Method* m);
-- 
cgit v1.1