Shrink the size of the VALUE_PAIR structure

Now that we're not writing strings to integer attributes,
we can dynamically change the size of the VALUE_PAIR.
It should be large enough to contain it's necessary fields,
and *only* enough of the VALUE_PAIR_DATA structure to contain
the type-specific data.  This means we save 250 bytes
of memory for every integer / date / ipaddr VALUE_PAIR.

diff --git a/src/lib/radius.c b/src/lib/radius.c
index ae458e0063ddf77159e0c8d1e0053ee1ee0e2223..3453ff9897ab45244952eb1e83d8838ddd99bc34 100644 (file)
--- a/src/lib/radius.c
+++ b/src/lib/radius.c
@@ -2814,6 +2814,7 @@ static ssize_t data2vp_any(const RADIUS_PACKET *packet,
        int data_offset = 0;
        DICT_ATTR *da;
        VALUE_PAIR *vp = NULL;
+       uint8_t buffer[256];
 
        if (length == 0) {
                /*
@@ -2912,8 +2913,8 @@ static ssize_t data2vp_any(const RADIUS_PACKET *packet,
        /*
         *      Copy the data to be decrypted
         */
-       vp->length = length - data_offset;
-       memcpy(&vp->vp_octets[0], data + data_offset, vp->length);
+       vp->length = length - data_offset;      
+       memcpy(buffer, data + data_offset, vp->length);
 
        /*
         *      Decrypt the attribute.
@@ -2924,16 +2925,17 @@ static ssize_t data2vp_any(const RADIUS_PACKET *packet,
                 */
        case FLAG_ENCRYPT_USER_PASSWORD:
                if (original) {
-                       rad_pwdecode(vp->vp_strvalue,
+                       rad_pwdecode(buffer,
                                     vp->length, secret,
                                     original->vector);
                } else {
-                       rad_pwdecode(vp->vp_strvalue,
+                       rad_pwdecode(buffer,
                                     vp->length, secret,
                                     packet->vector);
                }
+               buffer[253] = '\0';
                if (vp->attribute == PW_USER_PASSWORD) {
-                       vp->length = strlen(vp->vp_strvalue);
+                       vp->length = strlen(buffer);
                }
                break;
 
@@ -2944,7 +2946,7 @@ static ssize_t data2vp_any(const RADIUS_PACKET *packet,
        case FLAG_ENCRYPT_TUNNEL_PASSWORD:
                if (!original) goto raw;
 
-               if (rad_tunnel_pwdecode(vp->vp_octets, &vp->length,
+               if (rad_tunnel_pwdecode(buffer, &vp->length,
                                        secret, original->vector) < 0) {
                        goto raw;
                }
@@ -2962,10 +2964,10 @@ static ssize_t data2vp_any(const RADIUS_PACKET *packet,
                        make_secret(my_digest,
                                    original->vector,
                                    secret, data);
-                       memcpy(vp->vp_strvalue, my_digest,
+                       memcpy(buffer, my_digest,
                               AUTH_VECTOR_LEN );
-                       vp->vp_strvalue[AUTH_VECTOR_LEN] = '\0';
-                       vp->length = strlen(vp->vp_strvalue);
+                       buffer[AUTH_VECTOR_LEN] = '\0';
+                       vp->length = strlen(buffer);
                }
                break;
 
@@ -2976,58 +2978,49 @@ static ssize_t data2vp_any(const RADIUS_PACKET *packet,
 
        switch (vp->type) {
        case PW_TYPE_STRING:
+               memcpy(vp->vp_strvalue, buffer, vp->length);
+               vp->vp_strvalue[vp->length] = '\0';
+               break;
+
        case PW_TYPE_OCTETS:
        case PW_TYPE_ABINARY:
-               /* nothing more to do */
+               memcpy(vp->vp_octets, buffer, vp->length);
                break;
 
        case PW_TYPE_BYTE:
                if (vp->length != 1) goto raw;
 
-               vp->vp_integer = vp->vp_octets[0];
+               vp->vp_integer = buffer[0];
                break;
 
 
        case PW_TYPE_SHORT:
                if (vp->length != 2) goto raw;
 
-               vp->vp_integer = (vp->vp_octets[0] << 8) | vp->vp_octets[1];
+               vp->vp_integer = (buffer[0] << 8) | buffer[1];
                break;
 
        case PW_TYPE_INTEGER:
                if (vp->length != 4) goto raw;
 
-               memcpy(&vp->vp_integer, vp->vp_octets, 4);
+               memcpy(&vp->vp_integer, buffer, 4);
                vp->vp_integer = ntohl(vp->vp_integer);
 
                if (vp->flags.has_tag) vp->vp_integer &= 0x00ffffff;
-
-               /*
-                *      Try to get named VALUEs
-                */
-               {
-                       DICT_VALUE *dval;
-                       dval = dict_valbyattr(vp->attribute, vp->vendor,
-                                             vp->vp_integer);
-                       if (dval) {
-                               strlcpy(vp->vp_strvalue,
-                                       dval->name,
-                                       sizeof(vp->vp_strvalue));
-                       }
-               }
                break;
 
        case PW_TYPE_INTEGER64:
                if (vp->length != 8) goto raw;
 
                /* vp_integer64 is a union with vp_octets */
+               memcpy(&vp->vp_integer64, buffer, 8);
                vp->vp_integer64 = ntohll(vp->vp_integer64);
                break;
 
        case PW_TYPE_DATE:
                if (vp->length != 4) goto raw;
 
-               memcpy(&vp->vp_date, vp->vp_octets, 4);
+               memcpy(&vp->vp_date, buffer, 4);
                vp->vp_date = ntohl(vp->vp_date);
                break;
 
@@ -3035,7 +3028,7 @@ static ssize_t data2vp_any(const RADIUS_PACKET *packet,
        case PW_TYPE_IPADDR:
                if (vp->length != 4) goto raw;
 
-               memcpy(&vp->vp_ipaddr, vp->vp_octets, 4);
+               memcpy(&vp->vp_ipaddr, buffer, 4);
                break;
 
                /*
@@ -3043,7 +3036,7 @@ static ssize_t data2vp_any(const RADIUS_PACKET *packet,
                 */
        case PW_TYPE_IFID:
                if (vp->length != 8) goto raw;
-               /* vp->vp_ifid == vp->vp_octets */
+               memcpy(&vp->vp_ifid, buffer, 8);
                break;
 
                /*
@@ -3051,7 +3044,7 @@ static ssize_t data2vp_any(const RADIUS_PACKET *packet,
                 */
        case PW_TYPE_IPV6ADDR:
                if (vp->length != 16) goto raw;
-               /* vp->vp_ipv6addr == vp->vp_octets */
+               memcpy(&vp->vp_ipv6addr, buffer, 16);
                break;
 
                /*
@@ -3065,14 +3058,15 @@ static ssize_t data2vp_any(const RADIUS_PACKET *packet,
                 */
        case PW_TYPE_IPV6PREFIX:
                if (vp->length < 2 || vp->length > 18) goto raw;
-               if (vp->vp_octets[1] > 128) goto raw;
+               if (buffer[1] > 128) goto raw;
 
                /*
                 *      FIXME: double-check that
                 *      (vp->vp_octets[1] >> 3) matches vp->length + 2
                 */
+               memcpy(&vp->vp_ipv6prefix, buffer, vp->length);
                if (vp->length < 18) {
-                       memset(vp->vp_octets + vp->length, 0,
+                       memset(((uint8_t *)vp->vp_ipv6prefix) + vp->length, 0,
                               18 - vp->length);
                }
                break;
@@ -3084,9 +3078,8 @@ static ssize_t data2vp_any(const RADIUS_PACKET *packet,
                 *      Overload vp_integer for ntohl, which takes
                 *      uint32_t, not int32_t
                 */
-               memcpy(&vp->vp_integer, vp->vp_octets, 4);
+               memcpy(&vp->vp_integer, buffer, 4);
                vp->vp_integer = ntohl(vp->vp_integer);
-               memcpy(&vp->vp_signed, &vp->vp_integer, 4);
                break;
 
        case PW_TYPE_TLV:
@@ -3097,12 +3090,12 @@ static ssize_t data2vp_any(const RADIUS_PACKET *packet,
        case PW_TYPE_COMBO_IP:
                if (vp->length == 4) {
                        vp->type = PW_TYPE_IPADDR;
-                       memcpy(&vp->vp_ipaddr, vp->vp_octets, 4);
+                       memcpy(&vp->vp_ipaddr, buffer, 4);
                        break;
 
                } else if (vp->length == 16) {
                        vp->type = PW_TYPE_IPV6ADDR;
-                       /* vp->vp_ipv6addr == vp->vp_octets */
+                       memcpy(&vp->vp_ipv6addr, buffer, 16);
                        break;
 
                }

diff --git a/src/lib/valuepair.c b/src/lib/valuepair.c
index bf69f3a8f9db9092f2e0e977d503fe7b1ddc9c91..d32d05506101986434de5c63a5cc875d12f79432 100644 (file)
--- a/src/lib/valuepair.c
+++ b/src/lib/valuepair.c
@@ -62,23 +62,54 @@ static const char *months[] = {
 #define FR_VP_NAME_PAD (32)
 #define FR_VP_NAME_LEN (30)
 
+static const int typesize[] = {
+       sizeof(((VALUE_PAIR *)0)->vp_strvalue),                 /* string */
+       4,                      /* integer */
+       4,                      /* ipaddr */
+       4,                      /* date */
+       sizeof(((VALUE_PAIR *)0)->data),                        /* abinary */
+       sizeof(((VALUE_PAIR *)0)->vp_octets),                   /* octets */
+       sizeof(((VALUE_PAIR *)0)->vp_ifid),
+       sizeof(((VALUE_PAIR *)0)->vp_ipv6addr),
+       sizeof(((VALUE_PAIR *)0)->vp_ipv6prefix),
+       4,                      /* byte (integer internally) */
+       4,                      /* short (integer internally) */
+       sizeof(((VALUE_PAIR *)0)->vp_ether),
+       4,                      /* signed */
+       sizeof(((VALUE_PAIR *)0)->data),                        /* combo IP */
+       sizeof(((VALUE_PAIR *)0)->vp_tlv),
+       sizeof(((VALUE_PAIR *)0)->data),                        /* extended */
+       sizeof(((VALUE_PAIR *)0)->data),                        /* extended-flags */
+       sizeof(((VALUE_PAIR *)0)->data),                        /* evs */
+       8                       /* integer64 */
+};
+
 VALUE_PAIR *pairalloc(DICT_ATTR *da)
 {
        size_t name_len = 0;
+       size_t dsize;
        VALUE_PAIR *vp;
 
        /*
         *      Not in the dictionary: the name is allocated AFTER
         *      the VALUE_PAIR struct.
         */
-       if (!da) name_len = FR_VP_NAME_PAD;
+       if (da) {
+               dsize = typesize[da->type];
+               name_len = 0;
+       } else {
+               dsize = sizeof(vp->data);
+               name_len = FR_VP_NAME_PAD;
+       }
 
-       vp = malloc(sizeof(*vp) + name_len);
+       dsize += sizeof(*vp) - sizeof(vp->data) + name_len;
+       
+       vp = malloc(dsize);
        if (!vp) {
                fr_strerror_printf("Out of memory");
                return NULL;
        }
-       memset(vp, 0, sizeof(*vp));
+       memset(vp, 0, dsize);
 
        if (da) {
                vp->attribute = da->attr;
@@ -204,7 +235,7 @@ void pairbasicfree(VALUE_PAIR *pair)
 {
        if (pair->type == PW_TYPE_TLV) free(pair->vp_tlv);
        /* clear the memory here */
-       memset(pair, 0, sizeof(*pair));
+       memset(pair, 0, sizeof(*pair) - sizeof(pair->data));
        free(pair);
 }
 
@@ -338,22 +369,24 @@ void pairreplace(VALUE_PAIR **first, VALUE_PAIR *replace)
  */
 VALUE_PAIR *paircopyvp(const VALUE_PAIR *vp)
 {
-       size_t name_len;
+       size_t dsize, name_len;
        VALUE_PAIR *n;
 
        if (!vp) return NULL;
-       
+
        if (!vp->flags.unknown_attr) {
+               dsize = typesize[vp->type];
                name_len = 0;
        } else {
+               dsize = sizeof(vp->data);
                name_len = FR_VP_NAME_PAD;
        }
-       
-       if ((n = malloc(sizeof(*n) + name_len)) == NULL) {
-               fr_strerror_printf("out of memory");
-               return NULL;
-       }
-       memcpy(n, vp, sizeof(*n) + name_len);
+       dsize += sizeof(*vp) - sizeof(vp->data) + name_len;
+
+       n = malloc(dsize);
+       if (!n) return NULL;
+
+       memcpy(n, vp, dsize);
 
        /*
         *      Reset the name field to point to the NEW attribute,
@@ -903,15 +936,6 @@ VALUE_PAIR *pairparsevalue(VALUE_PAIR *vp, const char *value)
 
        if (!value) return NULL;
 
-       /*
-        *      Even for integers, dates and ip addresses we
-        *      keep the original string in vp->vp_strvalue.
-        */
-       if (vp->type != PW_TYPE_TLV) {
-               strlcpy(vp->vp_strvalue, value, sizeof(vp->vp_strvalue));
-               vp->length = strlen(vp->vp_strvalue);
-       }
-
        switch(vp->type) {
                case PW_TYPE_STRING:
                        /*
@@ -1144,7 +1168,7 @@ VALUE_PAIR *pairparsevalue(VALUE_PAIR *vp, const char *value)
                                }
 
                                vp->length = size >> 1;
-                               if (size > 2*sizeof(vp->vp_octets)) {
+                               if (size > 2 * sizeof(vp->vp_octets)) {
                                        vp->type |= PW_FLAG_LONG;
                                        us = vp->vp_tlv = malloc(vp->length);
                                        if (!us) {
@@ -1158,6 +1182,9 @@ VALUE_PAIR *pairparsevalue(VALUE_PAIR *vp, const char *value)
                                        fr_strerror_printf("Invalid hex data");
                                        return NULL;
                                }
+                       } else {
+                               strlcpy(vp->vp_octets, value, sizeof(vp->vp_octets));
+                               vp->length = strlen(value);
                        }
                        break;