Reduce type-punning in ASN1_TYPE

ASN1_TYPE is a union of a bunch of pointer types. Often it takes a
shorthand and accesses a->value.ptr directly. This is allowed in C, but
not C++. Writing the switch/case barely takes more code, so just write
it that way.

Along the way, extract the code for cleaning up an ASN1_TYPE from
tasn_fre.c. This is a small step towards being able to use crypto/asn1's
types without depending on the templates. ASN1_TYPE_free still, for now,
calls into the templates. That will be fixable once tasn_*.c are
rewritten a bit further.

This also removes the weird hack here ASN1_primitive_free (an internal
function) with NULL ASN1_ITEM secretly meant to partially clean up the
ASN1_TYPE. We can just call the function directly now.

Bug: 574
Change-Id: Ie2ba41418801a366ab2f12eccc01e8dadf82c33e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58126
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>

crypto/asn1/a_type.c

diff --git a/crypto/asn1/a_type.c b/crypto/asn1/a_type.c
index d59fdba..0c2fd13 100644
--- a/crypto/asn1/a_type.c
+++ b/crypto/asn1/a_type.c
@@ -65,31 +65,65 @@
 
 
 int ASN1_TYPE_get(const ASN1_TYPE *a) {
-  if (a->type == V_ASN1_BOOLEAN || a->type == V_ASN1_NULL ||
-      a->value.ptr != NULL) {
-    return a->type;
+  switch (a->type) {
+    case V_ASN1_NULL:
+    case V_ASN1_BOOLEAN:
+      return a->type;
+    case V_ASN1_OBJECT:
+      return a->value.object != NULL ? a->type : 0;
+    default:
+      return a->value.asn1_string != NULL ? a->type : 0;
   }
-  return 0;
 }
 
 const void *asn1_type_value_as_pointer(const ASN1_TYPE *a) {
-  if (a->type == V_ASN1_BOOLEAN) {
-    return a->value.boolean ? (void *)0xff : NULL;
+  switch (a->type) {
+    case V_ASN1_NULL:
+      return NULL;
+    case V_ASN1_BOOLEAN:
+      return a->value.boolean ? (void *)0xff : NULL;
+    case V_ASN1_OBJECT:
+      return a->value.object;
+    default:
+      return a->value.asn1_string;
   }
-  if (a->type == V_ASN1_NULL) {
-    return NULL;
+}
+
+void asn1_type_cleanup(ASN1_TYPE *a) {
+  switch (a->type) {
+    case V_ASN1_NULL:
+      a->value.ptr = NULL;
+      break;
+    case V_ASN1_BOOLEAN:
+      a->value.boolean = ASN1_BOOLEAN_NONE;
+      break;
+    case V_ASN1_OBJECT:
+      ASN1_OBJECT_free(a->value.object);
+      a->value.object = NULL;
+      break;
+    default:
+      ASN1_STRING_free(a->value.asn1_string);
+      a->value.asn1_string = NULL;
+      break;
   }
-  return a->value.ptr;
 }
 
 void ASN1_TYPE_set(ASN1_TYPE *a, int type, void *value) {
-  ASN1_TYPE **tmp_a = &a;
-  ASN1_primitive_free((ASN1_VALUE **)tmp_a, NULL);
+  asn1_type_cleanup(a);
   a->type = type;
-  if (type == V_ASN1_BOOLEAN) {
-    a->value.boolean = value ? 0xff : 0;
-  } else {
-    a->value.ptr = value;
+  switch (type) {
+    case V_ASN1_NULL:
+      a->value.ptr = NULL;
+      break;
+    case V_ASN1_BOOLEAN:
+      a->value.boolean = value ? ASN1_BOOLEAN_TRUE : ASN1_BOOLEAN_FALSE;
+      break;
+    case V_ASN1_OBJECT:
+      a->value.object = value;
+      break;
+    default:
+      a->value.asn1_string = value;
+      break;
   }
 }
 

crypto/asn1/internal.h

diff --git a/crypto/asn1/internal.h b/crypto/asn1/internal.h
index a60a037..64e1e6b 100644
--- a/crypto/asn1/internal.h
+++ b/crypto/asn1/internal.h
@@ -210,6 +210,10 @@
 // a pointer.
 const void *asn1_type_value_as_pointer(const ASN1_TYPE *a);
 
+// asn1_type_cleanup releases memory associated with |a|'s value, without
+// freeing |a| itself.
+void asn1_type_cleanup(ASN1_TYPE *a);
+
 // asn1_is_printable returns one if |value| is a valid Unicode codepoint for an
 // ASN.1 PrintableString, and zero otherwise.
 int asn1_is_printable(uint32_t value);

crypto/asn1/tasn_fre.c

diff --git a/crypto/asn1/tasn_fre.c b/crypto/asn1/tasn_fre.c
index ebfd3d6..add46f5 100644
--- a/crypto/asn1/tasn_fre.c
+++ b/crypto/asn1/tasn_fre.c
@@ -161,12 +161,10 @@
 }
 
 void ASN1_template_free(ASN1_VALUE **pval, const ASN1_TEMPLATE *tt) {
-  size_t i;
   if (tt->flags & ASN1_TFLG_SK_MASK) {
     STACK_OF(ASN1_VALUE) *sk = (STACK_OF(ASN1_VALUE) *)*pval;
-    for (i = 0; i < sk_ASN1_VALUE_num(sk); i++) {
-      ASN1_VALUE *vtmp;
-      vtmp = sk_ASN1_VALUE_value(sk, i);
+    for (size_t i = 0; i < sk_ASN1_VALUE_num(sk); i++) {
+      ASN1_VALUE *vtmp = sk_ASN1_VALUE_value(sk, i);
       ASN1_item_ex_free(&vtmp, ASN1_ITEM_ptr(tt->item));
     }
     sk_ASN1_VALUE_free(sk);
@@ -177,30 +175,11 @@
 }
 
 void ASN1_primitive_free(ASN1_VALUE **pval, const ASN1_ITEM *it) {
-  int utype;
   // Historically, |it->funcs| for primitive types contained an
   // |ASN1_PRIMITIVE_FUNCS| table of calbacks.
-  assert(it == NULL || it->funcs == NULL);
-  // Special case: if 'it' is NULL free contents of ASN1_TYPE
-  if (!it) {
-    ASN1_TYPE *typ = (ASN1_TYPE *)*pval;
-    utype = typ->type;
-    pval = &typ->value.asn1_value;
-    if (utype != V_ASN1_BOOLEAN && !*pval) {
-      return;
-    }
-  } else if (it->itype == ASN1_ITYPE_MSTRING) {
-    utype = -1;
-    if (!*pval) {
-      return;
-    }
-  } else {
-    utype = it->utype;
-    if ((utype != V_ASN1_BOOLEAN) && !*pval) {
-      return;
-    }
-  }
+  assert(it->funcs == NULL);
 
+  int utype = it->itype == ASN1_ITYPE_MSTRING ? -1 : it->utype;
   switch (utype) {
     case V_ASN1_OBJECT:
       ASN1_OBJECT_free((ASN1_OBJECT *)*pval);
@@ -210,7 +189,7 @@
       if (it) {
         *(ASN1_BOOLEAN *)pval = (ASN1_BOOLEAN)it->size;
       } else {
-        *(ASN1_BOOLEAN *)pval = -1;
+        *(ASN1_BOOLEAN *)pval = ASN1_BOOLEAN_NONE;
       }
       return;
 
@@ -218,8 +197,10 @@
       break;
 
     case V_ASN1_ANY:
-      ASN1_primitive_free(pval, NULL);
-      OPENSSL_free(*pval);
+      if (*pval != NULL) {
+        asn1_type_cleanup((ASN1_TYPE *)*pval);
+        OPENSSL_free(*pval);
+      }
       break;
 
     default: