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 <rsalz@openssl.org>

diff --git a/ssl/packet.c b/ssl/packet.c
index 52c24769f9d73eec261d6820486c06628727d59f..7d80ebc689096c2193bb21966669fa8a89028cc6 100644 (file)
--- 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;
 }
 
     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;
             || !WPACKET_allocate_bytes(pkt, len, allocbytes)
             || !WPACKET_close(pkt))
         return 0;


@@ -198,7 +198,7 @@ int WPACKET_finish(WPACKET *pkt)
     return ret;
 }
 
     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;
 {
     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)
 {
 
 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)
 }
 
 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;
 }
 
     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;
             || !WPACKET_memcpy(pkt, src, len)
             || !WPACKET_close(pkt))
         return 0;

diff --git a/ssl/packet_locl.h b/ssl/packet_locl.h
index 6f16a7aaf1d1e38b4c72e77d01eee186754213a9..0ec5a389cebbb16b4a05780c7961e2cc46fc4953 100644 (file)
--- 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
 
 /*
  * 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) \
 
 /*
  * 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) \
 #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) \
 #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) \
 #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);
 
  */
 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
 /*
  * 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) \
 
 /*
  * 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) \
 #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) \
 #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) \
 #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
 
 /*
  * 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
 
 /*
  * 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) \
                        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) \
 #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) \
 #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) \
 #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
 
 /*
  * Return the total number of bytes written so far to the underlying buffer

diff --git a/test/wpackettest.c b/test/wpackettest.c
index 0e08d950fa25ebd758fa6282b3fa9462cc0ed0e0..ac712b46f9e6ea274d16ab1043c7a4e8b56ad96e 100644 (file)
--- 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)
 
    /* 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)
             || !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)
 
     /* 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_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
             || !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)
 
     /* 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_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)
             || !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)
 
     /* 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)
             || !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)
 
     /* 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)
             || !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)
 
     /* 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;
     }
         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)
 
     /* 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)
             || !WPACKET_finish(&pkt)
             || !WPACKET_get_total_written(&pkt, &written)
             ||  written != sizeof(submem)