Comments; Better documentation and clearer logic

crypto/ec_extra/ec_asn1.c

@@ -615,43 +615,46 @@ BIGNUM *EC_POINT_point2bn(const EC_GROUP *group, const EC_POINT *point,
   return ret;
 }
 
-EC_POINT *EC_POINT_bn2point(const EC_GROUP *group,
-                            const BIGNUM *bn, EC_POINT *point, BN_CTX *ctx) {
-  EC_POINT *ret = NULL;
-
+EC_POINT *EC_POINT_bn2point(const EC_GROUP *group, const BIGNUM *bn,
+                            EC_POINT *point, BN_CTX *ctx) {
   // Allocate buffer and length.
   size_t buf_len = BN_num_bytes(bn);
   if (buf_len == 0) {
+    // See https://github.com/openssl/openssl/issues/10258
     buf_len = 1;
   }
   uint8_t *buf = OPENSSL_malloc(buf_len);
   if (buf == NULL) {
     return NULL;
   }
 
-  if (BN_bn2bin_padded(buf, buf_len,bn) < 0) {
-    goto end;
+  if (BN_bn2bin_padded(buf, buf_len, bn) < 0) {
+    OPENSSL_free(buf);
+    return NULL;
   }
 
-  // Allocate new |EC_POINT| if |point| is NULL. Otherwise, use |point|.
-  if (point == NULL) {
+  // Use the user-provided |point| if there is one. Otherwise, we allocate a new
+  // |EC_POINT| if |point| is NULL.
+  EC_POINT *ret;
+  if (point != NULL) {
+    ret = point;
+  } else {
     ret = EC_POINT_new(group);
     if (ret == NULL) {
-      goto end;
+      OPENSSL_free(buf);
+      return NULL;
     }
-  } else {
-    ret = point;
   }
 
   if (!EC_POINT_oct2point(group, ret, buf, buf_len, ctx)) {
     if (ret != point) {
-      // Free the newly allocated |EC_POINT| on failure.
+      // If the user did not provide a |point|, we free the |EC_POINT| we
+      // allocated.
       EC_POINT_free(ret);
     }
-    goto end;
   }
 
-end:
   OPENSSL_free(buf);
   return ret;
 }
+

include/openssl/ec.h

@@ -392,19 +392,28 @@ OPENSSL_EXPORT int EC_GROUP_set_generator(EC_GROUP *group,
                                           const BIGNUM *cofactor);
 
 
-// EC_POINT_point2bn converts an |EC_POINT| to a |BIGNUM| by serializing the
-// point into the X9.62 form given by |form| then interpreting it as a BIGNUM.
+// EC_POINT_point2bn serialises |point| into the X9.62 form given by |form|
+// and returns the |BIGNUM| representation of the serialised output.
 // On success, it returns the BIGNUM pointer supplied or, if |ret| is NULL,
 // allocates and returns a fresh |BIGNUM|. On error, it returns NULL. The |ctx|
 // argument may be used if not NULL.
+//
+// Note: |EC_POINT| serialization formats are not individual big-endian
+// integers, so these aren't particularly useful. Use |EC_POINT_point2oct|
+// instead.
 OPENSSL_EXPORT OPENSSL_DEPRECATED BIGNUM *EC_POINT_point2bn(
     const EC_GROUP *group, const EC_POINT *point, point_conversion_form_t form,
     BIGNUM *ret, BN_CTX *ctx);
 
-// EC_POINT_bn2point is like |EC_POINT_point2bn|, but converts a |BIGNUM| to an
-// |EC_POINT| instead. On success, it returns the EC_POINT pointer supplied or,
-// if |ret| is NULL, allocates and returns a fresh |EC_POINT|. On error, it
-// returns NULL. The |ctx| argument may be used if not NULL.
+// EC_POINT_bn2point is like |EC_POINT_point2bn|, but takes the |BIGNUM|
+// representation and de-serialises it back to an |EC_POINT|. On success,
+// it returns the EC_POINT pointer supplied or, if |ret| is NULL, allocates and
+// returns a fresh |EC_POINT|. On error, it returns NULL. The |ctx| argument
+// may be used if not NULL.
+//
+// Note: |EC_POINT| serialization formats are not individual big-endian
+// integers, so these aren't particularly useful. Use |EC_POINT_oct2point|
+// instead.
 OPENSSL_EXPORT OPENSSL_DEPRECATED EC_POINT *EC_POINT_bn2point(
     const EC_GROUP *group, const BIGNUM *bn, EC_POINT *point, BN_CTX *ctx);