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

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,
+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

@@ -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

@@ -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)