Commit 869d0a37 authored by Matt Caswell's avatar Matt Caswell
Browse files

Encourage use of the macros for the various "sub" functions 



Don't call WPACKET_sub_memcpy(), WPACKET_sub_allocation_bytes() and
WPACKET_start_sub_packet_len() directly.

Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
parent c9216d14

ssl/packet.c

+8 −7
Original line number Diff line number Diff line
@@ -41,10 +41,10 @@ int WPACKET_allocate_bytes(WPACKET *pkt, size_t len, unsigned char **allocbytes) 
    return 1;

}



int WPACKET_sub_allocate_bytes(WPACKET *pkt, size_t len,

int WPACKET_sub_allocate_bytes__(WPACKET *pkt, size_t len,

                                 unsigned char **allocbytes, size_t lenbytes)

{

    if (!WPACKET_start_sub_packet_len(pkt, lenbytes)

    if (!WPACKET_start_sub_packet_len__(pkt, lenbytes)

            || !WPACKET_allocate_bytes(pkt, len, allocbytes)

            || !WPACKET_close(pkt))

        return 0;
@@ -198,7 +198,7 @@ int WPACKET_finish(WPACKET *pkt) 
    return ret;

}



int WPACKET_start_sub_packet_len(WPACKET *pkt, size_t lenbytes)

int WPACKET_start_sub_packet_len__(WPACKET *pkt, size_t lenbytes)

{

    WPACKET_SUB *sub;

    unsigned char *lenchars;
@@ -231,7 +231,7 @@ int WPACKET_start_sub_packet_len(WPACKET *pkt, size_t lenbytes) 


int WPACKET_start_sub_packet(WPACKET *pkt)

{

    return WPACKET_start_sub_packet_len(pkt, 0);

    return WPACKET_start_sub_packet_len__(pkt, 0);

}



int WPACKET_put_bytes(WPACKET *pkt, unsigned int val, size_t size)
@@ -290,9 +290,10 @@ int WPACKET_memcpy(WPACKET *pkt, const void *src, size_t len) 
    return 1;

}



int WPACKET_sub_memcpy(WPACKET *pkt, const void *src, size_t len, size_t lenbytes)

int WPACKET_sub_memcpy__(WPACKET *pkt, const void *src, size_t len,

                         size_t lenbytes)

{

    if (!WPACKET_start_sub_packet_len(pkt, lenbytes)

    if (!WPACKET_start_sub_packet_len__(pkt, lenbytes)

            || !WPACKET_memcpy(pkt, src, len)

            || !WPACKET_close(pkt))

        return 0;

ssl/packet_locl.h

+24 −21
Original line number Diff line number Diff line
@@ -643,26 +643,27 @@ int WPACKET_finish(WPACKET *pkt); 


/*

 * Initialise a new sub-packet. Additionally |lenbytes| of data is preallocated

 * at the start of the sub-packet to store its length once we know it.

 * at the start of the sub-packet to store its length once we know it. Don't

 * call this directly. Use the convenience macros below instead.

 */

int WPACKET_start_sub_packet_len(WPACKET *pkt, size_t lenbytes);

int WPACKET_start_sub_packet_len__(WPACKET *pkt, size_t lenbytes);



/*

 * Convenience macros for calling WPACKET_start_sub_packet_len with different

 * lengths

 */

#define WPACKET_start_sub_packet_u8(pkt) \

    WPACKET_start_sub_packet_len((pkt), 1)

    WPACKET_start_sub_packet_len__((pkt), 1)

#define WPACKET_start_sub_packet_u16(pkt) \

    WPACKET_start_sub_packet_len((pkt), 2)

    WPACKET_start_sub_packet_len__((pkt), 2)

#define WPACKET_start_sub_packet_u24(pkt) \

    WPACKET_start_sub_packet_len((pkt), 3)

    WPACKET_start_sub_packet_len__((pkt), 3)

#define WPACKET_start_sub_packet_u32(pkt) \

    WPACKET_start_sub_packet_len((pkt), 4)

    WPACKET_start_sub_packet_len__((pkt), 4)



/*

 * Same as WPACKET_start_sub_packet_len() except no bytes are pre-allocated for

 * the sub-packet length.

 * Same as WPACKET_start_sub_packet_len__() except no bytes are pre-allocated

 * for the sub-packet length.

 */

int WPACKET_start_sub_packet(WPACKET *pkt);
@@ -677,9 +678,10 @@ int WPACKET_allocate_bytes(WPACKET *pkt, size_t bytes, 
/*

 * The same as WPACKET_allocate_bytes() except additionally a new sub-packet is

 * started for the allocated bytes, and then closed immediately afterwards. The

 * number of length bytes for the sub-packet is in |lenbytes|.

 * number of length bytes for the sub-packet is in |lenbytes|. Don't call this

 * directly. Use the convenience macros below instead.

 */

int WPACKET_sub_allocate_bytes(WPACKET *pkt, size_t len,

int WPACKET_sub_allocate_bytes__(WPACKET *pkt, size_t len,

                                 unsigned char **allocbytes, size_t lenbytes);



/*
@@ -687,13 +689,13 @@ int WPACKET_sub_allocate_bytes(WPACKET *pkt, size_t len, 
 * lengths

 */

#define WPACKET_sub_allocate_bytes_u8(pkt, len, bytes) \

    WPACKET_sub_allocate_bytes((pkt), (len), (bytes), 1)

    WPACKET_sub_allocate_bytes__((pkt), (len), (bytes), 1)

#define WPACKET_sub_allocate_bytes_u16(pkt, len, bytes) \

    WPACKET_sub_allocate_bytes((pkt), (len), (bytes), 2)

    WPACKET_sub_allocate_bytes__((pkt), (len), (bytes), 2)

#define WPACKET_sub_allocate_bytes_u24(pkt, len, bytes) \

    WPACKET_sub_allocate_bytes((pkt), (len), (bytes), 3)

    WPACKET_sub_allocate_bytes__((pkt), (len), (bytes), 3)

#define WPACKET_sub_allocate_bytes_u32(pkt, len, bytes) \

    WPACKET_sub_allocate_bytes((pkt), (len), (bytes), 4)

    WPACKET_sub_allocate_bytes__((pkt), (len), (bytes), 4)



/*

 * Write the value stored in |val| into the WPACKET. The value will consume
@@ -711,20 +713,21 @@ int WPACKET_memcpy(WPACKET *pkt, const void *src, size_t len); 


/*

 * Copy |len| bytes of data from |*src| into the WPACKET and prefix with its

 * length (consuming |lenbytes| of data for the length)

 * length (consuming |lenbytes| of data for the length). Don't call this

 * directly. Use the convenience macros below instead.

 */

int WPACKET_sub_memcpy(WPACKET *pkt, const void *src, size_t len,

int WPACKET_sub_memcpy__(WPACKET *pkt, const void *src, size_t len,

                       size_t lenbytes);



/* Convenience macros for calling WPACKET_sub_memcpy with different lengths */

#define WPACKET_sub_memcpy_u8(pkt, src, len) \

    WPACKET_sub_memcpy((pkt), (src), (len), 1)

    WPACKET_sub_memcpy__((pkt), (src), (len), 1)

#define WPACKET_sub_memcpy_u16(pkt, src, len) \

    WPACKET_sub_memcpy((pkt), (src), (len), 2)

    WPACKET_sub_memcpy__((pkt), (src), (len), 2)

#define WPACKET_sub_memcpy_bytes_u24(pkt, src, len) \

    WPACKET_sub_memcpy((pkt), (src), (len), 3)

    WPACKET_sub_memcpy__((pkt), (src), (len), 3)

#define WPACKET_sub_memcpy_bytes_u32(pkt, src, len) \

    WPACKET_sub_memcpy((pkt), (src), (len), 4)

    WPACKET_sub_memcpy__((pkt), (src), (len), 4)



/*

 * Return the total number of bytes written so far to the underlying buffer

test/wpackettest.c

+9 −9
Original line number Diff line number Diff line
@@ -179,7 +179,7 @@ static int test_WPACKET_start_sub_packet(void) 


   /* Single sub-packet with length prefix */

    if (!WPACKET_init(&pkt, buf)

            || !WPACKET_start_sub_packet_len(&pkt, 1)

            || !WPACKET_start_sub_packet_u8(&pkt)

            || !WPACKET_put_bytes(&pkt, 0xff, 1)

            || !WPACKET_close(&pkt)

            || !WPACKET_finish(&pkt)
@@ -192,9 +192,9 @@ static int test_WPACKET_start_sub_packet(void) 


    /* Nested sub-packets with length prefixes */

    if (!WPACKET_init(&pkt, buf)

            || !WPACKET_start_sub_packet_len(&pkt, 1)

            || !WPACKET_start_sub_packet_u8(&pkt)

            || !WPACKET_put_bytes(&pkt, 0xff, 1)

            || !WPACKET_start_sub_packet_len(&pkt, 1)

            || !WPACKET_start_sub_packet_u8(&pkt)

            || !WPACKET_put_bytes(&pkt, 0xff, 1)

            || !WPACKET_get_length(&pkt, &len)

            || len != 1
@@ -212,10 +212,10 @@ static int test_WPACKET_start_sub_packet(void) 


    /* Sequential sub-packets with length prefixes */

    if (!WPACKET_init(&pkt, buf)

            || !WPACKET_start_sub_packet_len(&pkt, 1)

            || !WPACKET_start_sub_packet_u8(&pkt)

            || !WPACKET_put_bytes(&pkt, 0xff, 1)

            || !WPACKET_close(&pkt)

            || !WPACKET_start_sub_packet_len(&pkt, 1)

            || !WPACKET_start_sub_packet_u8(&pkt)

            || !WPACKET_put_bytes(&pkt, 0xff, 1)

            || !WPACKET_close(&pkt)

            || !WPACKET_finish(&pkt)
@@ -277,7 +277,7 @@ static int test_WPACKET_set_flags(void) 


    /* Repeat above test but only abandon a sub-packet */

    if (!WPACKET_init_len(&pkt, buf, 1)

            || !WPACKET_start_sub_packet_len(&pkt, 1)

            || !WPACKET_start_sub_packet_u8(&pkt)

            || !WPACKET_set_flags(&pkt, WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH)

            || !WPACKET_close(&pkt)

            || !WPACKET_finish(&pkt)
@@ -290,7 +290,7 @@ static int test_WPACKET_set_flags(void) 


    /* And repeat with a non empty sub-packet */

    if (!WPACKET_init(&pkt, buf)

            || !WPACKET_start_sub_packet_len(&pkt, 1)

            || !WPACKET_start_sub_packet_u8(&pkt)

            || !WPACKET_set_flags(&pkt, WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH)

            || !WPACKET_put_bytes(&pkt, 0xff, 1)

            || !WPACKET_close(&pkt)
@@ -327,7 +327,7 @@ static int test_WPACKET_allocate_bytes(void) 


    /* Repeat with WPACKET_sub_allocate_bytes */

    if (!WPACKET_init_len(&pkt, buf, 1)

            || !WPACKET_sub_allocate_bytes(&pkt, 2, &bytes, 1)) {

            || !WPACKET_sub_allocate_bytes_u8(&pkt, 2, &bytes)) {

        testfail("test_WPACKET_allocate_bytes():3 failed\n", &pkt);

        return 0;

    }
@@ -362,7 +362,7 @@ static int test_WPACKET_memcpy(void) 


    /* Repeat with WPACKET_sub_memcpy() */

    if (!WPACKET_init_len(&pkt, buf, 1)

            || !WPACKET_sub_memcpy(&pkt, bytes, sizeof(bytes), 1)

            || !WPACKET_sub_memcpy_u8(&pkt, bytes, sizeof(bytes))

            || !WPACKET_finish(&pkt)

            || !WPACKET_get_total_written(&pkt, &written)

            ||  written != sizeof(submem)