From 869d0a37cfa7cfdbd42026d2b75d14cdc64e81e0 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Tue, 13 Sep 2016 15:42:12 +0100 Subject: [PATCH] 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: Rich Salz --- ssl/packet.c | 15 ++++++++------- ssl/packet_locl.h | 45 ++++++++++++++++++++++++--------------------- test/wpackettest.c | 18 +++++++++--------- 3 files changed, 41 insertions(+), 37 deletions(-) diff --git a/ssl/packet.c b/ssl/packet.c index 52c24769f9..7d80ebc689 100644 --- a/ssl/packet.c +++ b/ssl/packet.c @@ -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, - unsigned char **allocbytes, size_t lenbytes) +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; diff --git a/ssl/packet_locl.h b/ssl/packet_locl.h index 6f16a7aaf1..0ec5a389ce 100644 --- a/ssl/packet_locl.h +++ b/ssl/packet_locl.h @@ -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,23 +678,24 @@ 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, - unsigned char **allocbytes, size_t lenbytes); +int WPACKET_sub_allocate_bytes__(WPACKET *pkt, size_t len, + unsigned char **allocbytes, size_t lenbytes); /* * Convenience macros for calling WPACKET_sub_allocate_bytes with different * 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 diff --git a/test/wpackettest.c b/test/wpackettest.c index 0e08d950fa..ac712b46f9 100644 --- a/test/wpackettest.c +++ b/test/wpackettest.c @@ -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) -- 2.25.1