Re: [PATCH] cleanup libssl/src/crypto/asn1/a_{int,enum}.c

[Miod Vallat]

> * remove unnecessary temp variable d
> * move loop counter j in for() header
> * fix prototype for memcpy
> * make calculation of actual length in BN_to_ASN1_ENUMERATED
>   more transparent
> 
> This code still looks rather odd, it uses a temporary buffer to first
> convert the number into a minimal little endian representation and then
> reverts the bytes to minimal big endian representation. A simple clz()
> could have revealed the number of bytes necessary to encode. I will fix
> this later.

I'm not too fond of this kind of change - the compiler does a good job
at merging or optimizing temporary variables.

On the other hand, I think this subset of your diff is worth putting in:

Index: a_enum.c
===================================================================
RCS file: /cvs/src/lib/libssl/src/crypto/asn1/a_enum.c,v
retrieving revision 1.12
diff -u -p -r1.12 a_enum.c
--- a_enum.c    21 Apr 2014 11:37:41 -0000      1.12
+++ a_enum.c    28 Apr 2014 21:03:56 -0000
@@ -152,9 +152,9 @@ BN_to_ASN1_ENUMERATED(BIGNUM *bn, ASN1_E
        else
                ret->type = V_ASN1_ENUMERATED;
        j = BN_num_bits(bn);
-       len = ((j == 0) ? 0 : ((j / 8) + 1));
-       if (ret->length < len + 4) {
-               unsigned char *new_data = realloc(ret->data, len + 4);
+       len = 4 + ((j == 0) ? 0 : ((j / 8) + 1));
+       if (ret->length < len) {
+               unsigned char *new_data = realloc(ret->data, len);
                if (!new_data) {
                        ASN1err(ASN1_F_BN_TO_ASN1_ENUMERATED, 
ERR_R_MALLOC_FAILURE);
                        goto err;
Index: a_int.c
===================================================================
RCS file: /cvs/src/lib/libssl/src/crypto/asn1/a_int.c,v
retrieving revision 1.18
diff -u -p -r1.18 a_int.c
--- a_int.c     21 Apr 2014 11:37:41 -0000      1.18
+++ a_int.c     28 Apr 2014 21:03:56 -0000
@@ -159,7 +159,7 @@ i2c_ASN1_INTEGER(ASN1_INTEGER *a, unsign
        if (a->length == 0)
                *(p++) = 0;
        else if (!neg)
-               memcpy(p, a->data, (unsigned int)a->length);
+               memcpy(p, a->data, (size_t)a->length);
        else {
                /* Begin at the end of the encoding */
                n = a->data + a->length - 1;
@@ -426,9 +426,9 @@ BN_to_ASN1_INTEGER(const BIGNUM *bn, ASN
        else
                ret->type = V_ASN1_INTEGER;
        j = BN_num_bits(bn);
-       len = ((j == 0) ? 0 : ((j / 8) + 1));
-       if (ret->length < len + 4) {
-               unsigned char *new_data = realloc(ret->data, len + 4);
+       len = 4 + ((j == 0) ? 0 : ((j / 8) + 1));
+       if (ret->length < len) {
+               unsigned char *new_data = realloc(ret->data, len);
                if (!new_data) {
                        ASN1err(ASN1_F_BN_TO_ASN1_INTEGER, 
ERR_R_MALLOC_FAILURE);
                        goto err;