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>
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;
}
}
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);
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:
