Avoid unnecessary memory allocation in string2hex()

Remove string2hex() and replace with newly added ossl_bin2hex(). Since
the output hex string is always returned to users as a String, we can
avoid the memory allocation by writing directly to the String buffer.
This also reduces some lines of code.

4 files changed, 56 insertions, 87 deletions

diff --git a/ext/openssl/ossl.c b/ext/openssl/ossl.c
index 5cd0fe21..a9000f25 100644
--- a/ext/openssl/ossl.c
+++ b/ext/openssl/ossl.c
@@ -12,40 +12,6 @@
 #include <ruby/thread_native.h> /* for OpenSSL < 1.1.0 locks */
 
 /*
- * String to HEXString conversion
- */
-int
-string2hex(const unsigned char *buf, int buf_len, char **hexbuf, int *hexbuf_len)
-{
-    static const char hex[]="0123456789abcdef";
-    int i, len;
-
-    if (buf_len < 0 || buf_len > INT_MAX / 2) { /* PARANOIA? */
-	return -1;
-    }
-    len = 2 * buf_len;
-    if (!hexbuf) { /* if no buf, return calculated len */
-	if (hexbuf_len) {
-	    *hexbuf_len = len;
-	}
-	return len;
-    }
-    if (!(*hexbuf = OPENSSL_malloc(len + 1))) {
-	return -1;
-    }
-    for (i = 0; i < buf_len; i++) {
-	(*hexbuf)[2 * i] = hex[((unsigned char)buf[i]) >> 4];
-	(*hexbuf)[2 * i + 1] = hex[buf[i] & 0x0f];
-    }
-    (*hexbuf)[2 * i] = '\0';
-
-    if (hexbuf_len) {
-	*hexbuf_len = len;
-    }
-    return len;
-}
-
-/*
  * Data Conversion
  */
 #define OSSL_IMPL_ARY2SK(name, type, expected_class, dup)	\
@@ -145,6 +111,21 @@ ossl_buf2str(char *buf, int len)
     return str;
 }
 
+void
+ossl_bin2hex(unsigned char *in, char *out, size_t inlen)
+{
+    const char *hex = "0123456789abcdef";
+    size_t i;
+
+    assert(inlen <= LONG_MAX / 2);
+    for (i = 0; i < inlen; i++) {
+	unsigned char p = in[i];
+
+	out[i * 2 + 0] = hex[p >> 4];
+	out[i * 2 + 1] = hex[p & 0x0f];
+    }
+}
+
 /*
  * our default PEM callback
  */
diff --git a/ext/openssl/ossl.h b/ext/openssl/ossl.h
index 0ba64e65..864d0683 100644
--- a/ext/openssl/ossl.h
+++ b/ext/openssl/ossl.h
@@ -97,11 +97,6 @@ extern VALUE eOSSLError;
 } while (0)
 
 /*
- * String to HEXString conversion
- */
-int string2hex(const unsigned char *, int, char **, int *);
-
-/*
  * Data Conversion
  */
 STACK_OF(X509) *ossl_x509_ary2sk0(VALUE);
@@ -118,6 +113,11 @@ do{\
     assert(newlen <= len);\
     rb_str_set_len((str), newlen);\
 }while(0)
+/*
+ * Convert binary string to hex string. The caller is responsible for
+ * ensuring out has (2 * len) bytes of capacity.
+ */
+void ossl_bin2hex(unsigned char *in, char *out, size_t len);
 
 /*
  * Our default PEM callback
diff --git a/ext/openssl/ossl_hmac.c b/ext/openssl/ossl_hmac.c
index ec3f9de7..270979ed 100644
--- a/ext/openssl/ossl_hmac.c
+++ b/ext/openssl/ossl_hmac.c
@@ -162,7 +162,7 @@ ossl_hmac_update(VALUE self, VALUE data)
 }
 
 static void
-hmac_final(HMAC_CTX *ctx, unsigned char **buf, unsigned int *buf_len)
+hmac_final(HMAC_CTX *ctx, unsigned char *buf, unsigned int *buf_len)
 {
     HMAC_CTX *final;
 
@@ -175,12 +175,7 @@ hmac_final(HMAC_CTX *ctx, unsigned char **buf, unsigned int *buf_len)
 	ossl_raise(eHMACError, "HMAC_CTX_copy");
     }
 
-    if (!(*buf = OPENSSL_malloc(HMAC_size(final)))) {
-	HMAC_CTX_free(final);
-	OSSL_Debug("Allocating %d mem", (int)HMAC_size(final));
-	ossl_raise(eHMACError, "Cannot allocate memory for hmac");
-    }
-    HMAC_Final(final, *buf, buf_len);
+    HMAC_Final(final, buf, buf_len);
     HMAC_CTX_free(final);
 }
 
@@ -191,26 +186,25 @@ hmac_final(HMAC_CTX *ctx, unsigned char **buf, unsigned int *buf_len)
  * Returns the authentication code an instance represents as a binary string.
  *
  * === Example
- *
- *	instance = OpenSSL::HMAC.new('key', OpenSSL::Digest.new('sha1'))
- * 	#=> f42bb0eeb018ebbd4597ae7213711ec60760843f
- * 	instance.digest
- * 	#=> "\xF4+\xB0\xEE\xB0\x18\xEB\xBDE\x97\xAEr\x13q\x1E\xC6\a`\x84?"
- *
+ *  instance = OpenSSL::HMAC.new('key', OpenSSL::Digest.new('sha1'))
+ *  #=> f42bb0eeb018ebbd4597ae7213711ec60760843f
+ *  instance.digest
+ *  #=> "\xF4+\xB0\xEE\xB0\x18\xEB\xBDE\x97\xAEr\x13q\x1E\xC6\a`\x84?"
  */
 static VALUE
 ossl_hmac_digest(VALUE self)
 {
     HMAC_CTX *ctx;
-    unsigned char *buf;
     unsigned int buf_len;
-    VALUE digest;
+    VALUE ret;
 
     GetHMAC(self, ctx);
-    hmac_final(ctx, &buf, &buf_len);
-    digest = ossl_buf2str((char *)buf, buf_len);
+    ret = rb_str_new(NULL, EVP_MAX_MD_SIZE);
+    hmac_final(ctx, (unsigned char *)RSTRING_PTR(ret), &buf_len);
+    assert(buf_len <= EVP_MAX_MD_SIZE);
+    rb_str_set_len(ret, buf_len);
 
-    return digest;
+    return ret;
 }
 
 /*
@@ -219,27 +213,21 @@ ossl_hmac_digest(VALUE self)
  *
  * Returns the authentication code an instance represents as a hex-encoded
  * string.
- *
  */
 static VALUE
 ossl_hmac_hexdigest(VALUE self)
 {
     HMAC_CTX *ctx;
-    unsigned char *buf;
-    char *hexbuf;
+    unsigned char buf[EVP_MAX_MD_SIZE];
     unsigned int buf_len;
-    VALUE hexdigest;
+    VALUE ret;
 
     GetHMAC(self, ctx);
-    hmac_final(ctx, &buf, &buf_len);
-    if (string2hex(buf, buf_len, &hexbuf, NULL) != 2 * (int)buf_len) {
-	OPENSSL_free(buf);
-	ossl_raise(eHMACError, "Memory alloc error");
-    }
-    OPENSSL_free(buf);
-    hexdigest = ossl_buf2str(hexbuf, 2 * buf_len);
+    hmac_final(ctx, buf, &buf_len);
+    ret = rb_str_new(NULL, buf_len * 2);
+    ossl_bin2hex(buf, RSTRING_PTR(ret), buf_len);
 
-    return hexdigest;
+    return ret;
 }
 
 /*
@@ -323,22 +311,22 @@ ossl_hmac_s_digest(VALUE klass, VALUE digest, VALUE key, VALUE data)
 static VALUE
 ossl_hmac_s_hexdigest(VALUE klass, VALUE digest, VALUE key, VALUE data)
 {
-    unsigned char *buf;
-    char *hexbuf;
+    unsigned char buf[EVP_MAX_MD_SIZE];
     unsigned int buf_len;
-    VALUE hexdigest;
+    VALUE ret;
 
     StringValue(key);
     StringValue(data);
 
-    buf = HMAC(GetDigestPtr(digest), RSTRING_PTR(key), RSTRING_LENINT(key),
-	       (unsigned char *)RSTRING_PTR(data), RSTRING_LEN(data), NULL, &buf_len);
-    if (string2hex(buf, buf_len, &hexbuf, NULL) != 2 * (int)buf_len) {
-	ossl_raise(eHMACError, "Cannot convert buf to hexbuf");
-    }
-    hexdigest = ossl_buf2str(hexbuf, 2 * buf_len);
+    if (!HMAC(GetDigestPtr(digest), RSTRING_PTR(key), RSTRING_LENINT(key),
+	      (unsigned char *)RSTRING_PTR(data), RSTRING_LEN(data),
+	      buf, &buf_len))
+	ossl_raise(eHMACError, "HMAC");
+
+    ret = rb_str_new(NULL, buf_len * 2);
+    ossl_bin2hex(buf, RSTRING_PTR(ret), buf_len);
 
-    return hexdigest;
+    return ret;
 }
 
 /*
diff --git a/ext/openssl/ossl_ocsp.c b/ext/openssl/ossl_ocsp.c
index d9ee51cd..a8b3503d 100644
--- a/ext/openssl/ossl_ocsp.c
+++ b/ext/openssl/ossl_ocsp.c
@@ -1602,15 +1602,15 @@ ossl_ocspcid_get_issuer_name_hash(VALUE self)
 {
     OCSP_CERTID *id;
     ASN1_OCTET_STRING *name_hash;
-    char *hexbuf;
+    VALUE ret;
 
     GetOCSPCertId(self, id);
     OCSP_id_get0_info(&name_hash, NULL, NULL, NULL, id);
 
-    if (string2hex(name_hash->data, name_hash->length, &hexbuf, NULL) < 0)
-	ossl_raise(eOCSPError, "string2hex");
+    ret = rb_str_new(NULL, name_hash->length * 2);
+    ossl_bin2hex(name_hash->data, RSTRING_PTR(ret), name_hash->length);
 
-    return ossl_buf2str(hexbuf, name_hash->length * 2);
+    return ret;
 }
 
 /*
@@ -1625,15 +1625,15 @@ ossl_ocspcid_get_issuer_key_hash(VALUE self)
 {
     OCSP_CERTID *id;
     ASN1_OCTET_STRING *key_hash;
-    char *hexbuf;
+    VALUE ret;
 
     GetOCSPCertId(self, id);
     OCSP_id_get0_info(NULL, NULL, &key_hash, NULL, id);
 
-    if (string2hex(key_hash->data, key_hash->length, &hexbuf, NULL) < 0)
-	ossl_raise(eOCSPError, "string2hex");
+    ret = rb_str_new(NULL, key_hash->length * 2);
+    ossl_bin2hex(key_hash->data, RSTRING_PTR(ret), key_hash->length);
 
-    return ossl_buf2str(hexbuf, key_hash->length * 2);
+    return ret;
 }
 
 /*