Address WPACKET review comments

A few style tweaks here and there. The main change is that curr and
packet_len are now offsets into the buffer to account for the fact that
the pointers can change if the buffer grows. Also dropped support for the
WPACKET_set_packet_len() function. I thought that was going to be needed
but so far it hasn't been. It doesn't really work any more due to the
offsets change.
Reviewed-by: NRich Salz <rsalz@openssl.org>

ssl/packet.c

@@ -25,23 +25,14 @@ int WPACKET_allocate_bytes(WPACKET *pkt, size_t len, unsigned char **allocbytes)
         if (pkt->buf->length > SIZE_MAX / 2) {
             newlen = SIZE_MAX;
         } else {
-            if (pkt->buf->length == 0)
-                newlen = DEFAULT_BUF_SIZE;
-            else
-                newlen = pkt->buf->length * 2;
+            newlen = (pkt->buf->length == 0) ? DEFAULT_BUF_SIZE
+                                             : pkt->buf->length * 2;
         }
         if (BUF_MEM_grow(pkt->buf, newlen) == 0)
             return 0;
-        if (pkt->curr == NULL) {
-            /*
-             * Can happen if initialised with a BUF_MEM that hasn't been
-             * pre-allocated.
-             */
-            pkt->curr = (unsigned char *)pkt->buf->data;
-        }
     }
     pkt->written += len;
-    *allocbytes = pkt->curr;
+    *allocbytes = (unsigned char *)pkt->buf->data + pkt->curr;
     pkt->curr += len;
 
     return 1;
@@ -57,12 +48,14 @@ static size_t maxmaxsize(size_t lenbytes)
 
 int WPACKET_init_len(WPACKET *pkt, BUF_MEM *buf, size_t lenbytes)
 {
+    unsigned char *lenchars;
+
     /* Sanity check */
     if (buf == NULL)
         return 0;
 
     pkt->buf = buf;
-    pkt->curr = (unsigned char *)buf->data;
+    pkt->curr = 0;
     pkt->written = 0;
     pkt->maxsize = maxmaxsize(lenbytes);
 
@@ -76,11 +69,12 @@ int WPACKET_init_len(WPACKET *pkt, BUF_MEM *buf, size_t lenbytes)
     pkt->subs->pwritten = lenbytes;
     pkt->subs->lenbytes = lenbytes;
 
-    if (!WPACKET_allocate_bytes(pkt, lenbytes, &(pkt->subs->packet_len))) {
+    if (!WPACKET_allocate_bytes(pkt, lenbytes, &lenchars)) {
         OPENSSL_free(pkt->subs);
         pkt->subs = NULL;
         return 0;
     }
+    pkt->subs->packet_len = lenchars - (unsigned char *)pkt->buf->data;
 
     return 1;
 }
@@ -90,25 +84,6 @@ int WPACKET_init(WPACKET *pkt, BUF_MEM *buf)
     return WPACKET_init_len(pkt, buf, 0);
 }
 
-int WPACKET_set_packet_len(WPACKET *pkt, unsigned char *packet_len,
-                           size_t lenbytes)
-{
-    size_t maxmax;
-
-    /* We only allow this to be set once */
-    if (pkt->subs == NULL || pkt->subs->lenbytes != 0)
-        return 0;
-
-    pkt->subs->lenbytes = lenbytes;
-    pkt->subs->packet_len = packet_len;
-
-    maxmax = maxmaxsize(lenbytes);
-    if (pkt->maxsize > maxmax)
-        pkt->maxsize = maxmax;
-
-    return 1;
-}
-
 int WPACKET_set_flags(WPACKET *pkt, unsigned int flags)
 {
     if (pkt->subs == NULL)
@@ -126,16 +101,15 @@ int WPACKET_set_flags(WPACKET *pkt, unsigned int flags)
  */
 static int wpacket_intern_close(WPACKET *pkt)
 {
-    size_t packlen;
     WPACKET_SUB *sub = pkt->subs;
+    size_t packlen = pkt->written - sub->pwritten;
 
-    packlen = pkt->written - sub->pwritten;
     if (packlen == 0
-            && sub->flags & OPENSSL_WPACKET_FLAGS_NON_ZERO_LENGTH)
+            && sub->flags & WPACKET_FLAGS_NON_ZERO_LENGTH)
         return 0;
 
     if (packlen == 0
-            && sub->flags & OPENSSL_WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH) {
+            && sub->flags & WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH) {
         /* Deallocate any bytes allocated for the length of the WPACKET */
         if ((pkt->curr - sub->lenbytes) == sub->packet_len) {
             pkt->written -= sub->lenbytes;
@@ -143,17 +117,16 @@ static int wpacket_intern_close(WPACKET *pkt)
         }
 
         /* Don't write out the packet length */
-        sub->packet_len = NULL;
+        sub->packet_len = 0;
+        sub->lenbytes = 0;
     }
 
     /* Write out the WPACKET length if needed */
-    if (sub->packet_len != NULL) {
+    if (sub->lenbytes > 0) {
         size_t lenbytes;
 
-        lenbytes = sub->lenbytes;
-
-        for (; lenbytes > 0; lenbytes--) {
-            sub->packet_len[lenbytes - 1]
+        for (lenbytes = sub->lenbytes; lenbytes > 0; lenbytes--) {
+            pkt->buf->data[sub->packet_len + lenbytes - 1]
                 = (unsigned char)(packlen & 0xff);
             packlen >>= 8;
         }
@@ -187,17 +160,18 @@ int WPACKET_finish(WPACKET *pkt)
         return 0;
 
     ret = wpacket_intern_close(pkt);
-
     if (ret) {
         OPENSSL_free(pkt->subs);
         pkt->subs = NULL;
     }
+
     return ret;
 }
 
 int WPACKET_start_sub_packet_len(WPACKET *pkt, size_t lenbytes)
 {
     WPACKET_SUB *sub;
+    unsigned char *lenchars;
 
     if (pkt->subs == NULL)
         return 0;
@@ -212,13 +186,13 @@ int WPACKET_start_sub_packet_len(WPACKET *pkt, size_t lenbytes)
     sub->lenbytes = lenbytes;
 
     if (lenbytes == 0) {
-        sub->packet_len = NULL;
+        sub->packet_len = 0;
         return 1;
     }
 
-    if (!WPACKET_allocate_bytes(pkt, lenbytes, &sub->packet_len)) {
+    if (!WPACKET_allocate_bytes(pkt, lenbytes, &lenchars))
         return 0;
-    }
+    sub->packet_len = lenchars - (unsigned char *)pkt->buf->data;
 
     return 1;
 }
@@ -228,16 +202,15 @@ int WPACKET_start_sub_packet(WPACKET *pkt)
     return WPACKET_start_sub_packet_len(pkt, 0);
 }
 
-int WPACKET_put_bytes(WPACKET *pkt, unsigned int val, size_t bytes)
+int WPACKET_put_bytes(WPACKET *pkt, unsigned int val, size_t size)
 {
     unsigned char *data;
 
-    if (bytes > sizeof(unsigned int)
-            || !WPACKET_allocate_bytes(pkt, bytes, &data))
+    if (size > sizeof(unsigned int)
+            || !WPACKET_allocate_bytes(pkt, size, &data))
         return 0;
 
-    data += bytes - 1;
-    for (; bytes > 0; bytes--) {
+    for (data += size - 1; size > 0; size--) {
         *data = (unsigned char)(val & 0xff);
         data--;
         val >>= 8;
@@ -259,7 +232,8 @@ int WPACKET_set_max_size(WPACKET *pkt, size_t maxsize)
         return 0;
 
     /* Find the WPACKET_SUB for the top level */
-    for (sub = pkt->subs; sub->parent != NULL; sub = sub->parent);
+    for (sub = pkt->subs; sub->parent != NULL; sub = sub->parent)
+        continue;
 
     lenbytes = sub->lenbytes;
     if (lenbytes == 0)

ssl/packet_locl.h

@@ -557,12 +557,12 @@ struct wpacket_sub {
     WPACKET_SUB *parent;
 
     /*
-     * Pointer to where the length of this WPACKET goes (or NULL if we don't
-     * write the length)
+     * Offset into the buffer where the length of this WPACKET goes. We use an
+     * offset in case the buffer grows and gets reallocated.
      */
-    unsigned char *packet_len;
+    size_t packet_len;
 
-    /* Number of bytes in the packet_len */
+    /* Number of bytes in the packet_len or 0 if we don't write the length */
     size_t lenbytes;
 
     /* Number of bytes written to the buf prior to this packet starting */
@@ -577,8 +577,11 @@ struct wpacket_st {
     /* The buffer where we store the output data */
     BUF_MEM *buf;
 
-    /* Pointer to where we are currently writing */
-    unsigned char *curr;
+    /*
+     * Offset into the buffer where we are currently writing. We use an offset
+     * in case the buffer grows and gets reallocated.
+     */
+    size_t curr;
 
     /* Number of bytes written so far */
     size_t written;
@@ -593,16 +596,16 @@ struct wpacket_st {
 /* Flags */
 
 /* Default */
-#define OPENSSL_WPACKET_FLAGS_NONE                      0
+#define WPACKET_FLAGS_NONE                      0
 
 /* Error on WPACKET_close() if no data written to the WPACKET */
-#define OPENSSL_WPACKET_FLAGS_NON_ZERO_LENGTH           1
+#define WPACKET_FLAGS_NON_ZERO_LENGTH           1
 
 /*
  * Abandon all changes on WPACKET_close() if no data written to the WPACKET,
  * i.e. this does not write out a zero packet length
  */
-#define OPENSSL_WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH    2
+#define WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH    2
 
 
 /*
@@ -624,17 +627,6 @@ int WPACKET_init(WPACKET *pkt, BUF_MEM *buf);
  */
 int WPACKET_set_flags(WPACKET *pkt, unsigned int flags);
 
-/*
- * Set the WPACKET length, and the location for where we should write that
- * length. Normally this will be at the start of the WPACKET, and therefore
- * the WPACKET would have been initialised via WPACKET_init_len(). However there
- * is the possibility that the length needs to be written to some other location
- * other than the start of the WPACKET. In that case init via WPACKET_init() and
- * then set the location for the length using this function.
- */
-int WPACKET_set_packet_len(WPACKET *pkt, unsigned char *packet_len,
-                           size_t lenbytes);
-
 /*
  * Closes the most recent sub-packet. It also writes out the length of the
  * packet to the required location (normally the start of the WPACKET) if
@@ -655,6 +647,19 @@ int WPACKET_finish(WPACKET *pkt);
  */
 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)
+#define WPACKET_start_sub_packet_u16(pkt) \
+    WPACKET_start_sub_packet_len((pkt), 2)
+#define WPACKET_start_sub_packet_u24(pkt) \
+    WPACKET_start_sub_packet_len((pkt), 3)
+#define WPACKET_start_sub_packet_u32(pkt) \
+    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.

ssl/s3_lib.c

@@ -2799,7 +2799,7 @@ int ssl3_set_handshake_header2(SSL *s, WPACKET *pkt, int htype)
 {
     /* Set the content type and 3 bytes for the message len */
     if (!WPACKET_put_bytes(pkt, htype, 1)
-            || !WPACKET_start_sub_packet_len(pkt, 3))
+            || !WPACKET_start_sub_packet_u24(pkt))
         return 0;
 
     return 1;

ssl/statem/statem_clnt.c

@@ -794,7 +794,7 @@ int tls_construct_client_hello(SSL *s)
     else
         i = s->session->session_id_length;
     if (i > (int)sizeof(s->session->session_id)
-            || !WPACKET_start_sub_packet_len(&pkt, 1)
+            || !WPACKET_start_sub_packet_u8(&pkt)
             || (i != 0 && !WPACKET_memcpy(&pkt, s->session->session_id, i))
             || !WPACKET_close(&pkt)) {
         SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, ERR_R_INTERNAL_ERROR);
@@ -812,7 +812,7 @@ int tls_construct_client_hello(SSL *s)
     }
 
     /* Ciphers supported */
-    if (!WPACKET_start_sub_packet_len(&pkt, 2)) {
+    if (!WPACKET_start_sub_packet_u16(&pkt)) {
         SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, ERR_R_INTERNAL_ERROR);
         goto err;
     }
@@ -825,7 +825,7 @@ int tls_construct_client_hello(SSL *s)
     }
 
     /* COMPRESSION */
-    if (!WPACKET_start_sub_packet_len(&pkt, 1)) {
+    if (!WPACKET_start_sub_packet_u8(&pkt)) {
         SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, ERR_R_INTERNAL_ERROR);
         goto err;
     }
@@ -852,13 +852,12 @@ int tls_construct_client_hello(SSL *s)
         SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, SSL_R_CLIENTHELLO_TLSEXT);
         goto err;
     }
-    if (!WPACKET_start_sub_packet_len(&pkt, 2)
+    if (!WPACKET_start_sub_packet_u16(&pkt)
                /*
                 * If extensions are of zero length then we don't even add the
                 * extensions length bytes
                 */
-            || !WPACKET_set_flags(&pkt,
-                                  OPENSSL_WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH)
+            || !WPACKET_set_flags(&pkt, WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH)
             || !ssl_add_clienthello_tlsext(s, &pkt, &al)
             || !WPACKET_close(&pkt)) {
         ssl3_send_alert(s, SSL3_AL_FATAL, al);

ssl/statem/statem_dtls.c

@@ -1200,6 +1200,7 @@ void dtls1_get_message_header(unsigned char *data, struct hm_header_st *msg_hdr)
 int dtls1_set_handshake_header2(SSL *s, WPACKET *pkt, int htype)
 {
     unsigned char *header;
+
     dtls1_set_message_header(s, htype, 0, 0, 0);
 
     /*

ssl/t1_ext.c

@@ -172,7 +172,7 @@ int custom_ext_add(SSL *s, int server, WPACKET *pkt, int *al)
         }
 
         if (!WPACKET_put_bytes(pkt, meth->ext_type, 2)
-                || !WPACKET_start_sub_packet_len(pkt, 2)
+                || !WPACKET_start_sub_packet_u16(pkt)
                 || (outlen > 0 && !WPACKET_memcpy(pkt, out, outlen))
                 || !WPACKET_close(pkt)) {
             *al = SSL_AD_INTERNAL_ERROR;

ssl/t1_lib.c

@@ -1054,9 +1054,9 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al)
         /* Add TLS extension servername to the Client Hello message */
         if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_server_name, 2)
                    /* Sub-packet for server_name extension */
-                || !WPACKET_start_sub_packet_len(pkt, 2)
+                || !WPACKET_start_sub_packet_u16(pkt)
                    /* Sub-packet for servername list (always 1 hostname)*/
-                || !WPACKET_start_sub_packet_len(pkt, 2)
+                || !WPACKET_start_sub_packet_u16(pkt)
                 || !WPACKET_put_bytes(pkt, TLSEXT_NAMETYPE_host_name, 1)
                 || !WPACKET_sub_memcpy(pkt, s->tlsext_hostname,
                                        strlen(s->tlsext_hostname), 2)
@@ -1071,11 +1071,10 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al)
     if (s->srp_ctx.login != NULL) {
         if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_srp, 2)
                    /* Sub-packet for SRP extension */
-                || !WPACKET_start_sub_packet_len(pkt, 2)
-                || !WPACKET_start_sub_packet_len(pkt, 1)
+                || !WPACKET_start_sub_packet_u16(pkt)
+                || !WPACKET_start_sub_packet_u8(pkt)
                    /* login must not be zero...internal error if so */
-                || !WPACKET_set_flags(pkt,
-                                      OPENSSL_WPACKET_FLAGS_NON_ZERO_LENGTH)
+                || !WPACKET_set_flags(pkt, WPACKET_FLAGS_NON_ZERO_LENGTH)
                 || !WPACKET_memcpy(pkt, s->srp_ctx.login,
                                    strlen(s->srp_ctx.login))
                 || !WPACKET_close(pkt)
@@ -1099,7 +1098,7 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al)
 
         if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_ec_point_formats, 2)
                    /* Sub-packet for formats extension */
-                || !WPACKET_start_sub_packet_len(pkt, 2)
+                || !WPACKET_start_sub_packet_u16(pkt)
                 || !WPACKET_sub_memcpy(pkt, pformats, num_formats, 1)
                 || !WPACKET_close(pkt)) {
             SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
@@ -1117,8 +1116,8 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al)
 
         if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_elliptic_curves, 2)
                    /* Sub-packet for curves extension */
-                || !WPACKET_start_sub_packet_len(pkt, 2)
-                || !WPACKET_start_sub_packet_len(pkt, 2)) {
+                || !WPACKET_start_sub_packet_u16(pkt)
+                || !WPACKET_start_sub_packet_u16(pkt)) {
             SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
             return 0;
         }
@@ -1178,9 +1177,9 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al)
 
         if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_signature_algorithms, 2)
                    /* Sub-packet for sig-algs extension */
-                || !WPACKET_start_sub_packet_len(pkt, 2)
+                || !WPACKET_start_sub_packet_u16(pkt)
                    /* Sub-packet for the actual list */
-                || !WPACKET_start_sub_packet_len(pkt, 2)
+                || !WPACKET_start_sub_packet_u16(pkt)
                 || !tls12_copy_sigalgs(s, pkt, salg, salglen)
                 || !WPACKET_close(pkt)
                 || !WPACKET_close(pkt)) {
@@ -1194,10 +1193,10 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al)
 
         if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_status_request, 2)
                    /* Sub-packet for status request extension */
-                || !WPACKET_start_sub_packet_len(pkt, 2)
+                || !WPACKET_start_sub_packet_u16(pkt)
                 || !WPACKET_put_bytes(pkt, TLSEXT_STATUSTYPE_ocsp, 1)
                    /* Sub-packet for the ids */
-                || !WPACKET_start_sub_packet_len(pkt, 2)) {
+                || !WPACKET_start_sub_packet_u16(pkt)) {
             SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
             return 0;
         }
@@ -1210,7 +1209,7 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al)
             idlen = i2d_OCSP_RESPID(id, NULL);
             if (idlen <= 0
                        /* Sub-packet for an individual id */
-                    || !WPACKET_start_sub_packet_len(pkt, 1)
+                    || !WPACKET_start_sub_packet_u8(pkt)
                     || !WPACKET_allocate_bytes(pkt, idlen, &idbytes)
                     || i2d_OCSP_RESPID(id, &idbytes) != idlen
                     || !WPACKET_close(pkt)) {
@@ -1219,7 +1218,7 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al)
             }
         }
         if (!WPACKET_close(pkt)
-                || !WPACKET_start_sub_packet_len(pkt, 2)) {
+                || !WPACKET_start_sub_packet_u16(pkt)) {
             SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
             return 0;
         }
@@ -1260,7 +1259,7 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al)
 
         if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_heartbeat, 2)
                    /* Sub-packet for Hearbeat extension */
-                || !WPACKET_start_sub_packet_len(pkt, 2)
+                || !WPACKET_start_sub_packet_u16(pkt)
                 || !WPACKET_put_bytes(pkt, mode, 1)
                 || !WPACKET_close(pkt)) {
             SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
@@ -1292,7 +1291,7 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al)
         if (!WPACKET_put_bytes(pkt,
                     TLSEXT_TYPE_application_layer_protocol_negotiation, 2)
                    /* Sub-packet ALPN extension */
-                || !WPACKET_start_sub_packet_len(pkt, 2)
+                || !WPACKET_start_sub_packet_u16(pkt)
                 || !WPACKET_sub_memcpy(pkt, s->alpn_client_proto_list,
                                        s->alpn_client_proto_list_len, 2)
                 || !WPACKET_close(pkt)) {
@@ -1309,9 +1308,9 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al)
 
         if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_use_srtp, 2)
                    /* Sub-packet for SRTP extension */
-                || !WPACKET_start_sub_packet_len(pkt, 2)
+                || !WPACKET_start_sub_packet_u16(pkt)
                    /* Sub-packet for the protection profile list */
-                || !WPACKET_start_sub_packet_len(pkt, 2)) {
+                || !WPACKET_start_sub_packet_u16(pkt)) {
             SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
             return 0;
         }
@@ -1381,7 +1380,7 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al)
                 hlen = 0;
 
             if (!WPACKET_put_bytes(pkt, TLSEXT_TYPE_padding, 2)
-                    || !WPACKET_start_sub_packet_len(pkt, 2)
+                    || !WPACKET_start_sub_packet_u16(pkt)
                     || !WPACKET_allocate_bytes(pkt, hlen, &padbytes)) {
                 SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
                 return 0;

test/wpackettest.c

@@ -98,45 +98,10 @@ static int test_WPACKET_init(void)
     return 1;
 }
 
-static int test_WPACKET_set_packet_len(void)
-{
-    WPACKET pkt;
-    size_t written;
-    unsigned char len;
-
-    /*
-     * Calling set_packet_len when the packet len is already set
-     * should fail
-     */
-    if (       !WPACKET_init_len(&pkt, buf, 1)
-            ||  WPACKET_set_packet_len(&pkt, &len, sizeof(len))
-            || !WPACKET_finish(&pkt)) {
-        testfail("test_WPACKET_set_packet_len():1 failed\n", &pkt);
-        return 0;
-    }
-
-    if (       !WPACKET_init(&pkt, buf)
-            || !WPACKET_set_packet_len(&pkt, &len, sizeof(len))
-                /* Can't set it again */
-            ||  WPACKET_set_packet_len(&pkt, &len, sizeof(len))
-            || !WPACKET_put_bytes(&pkt, 0xff, 1)
-            || !WPACKET_finish(&pkt)
-            || !WPACKET_get_total_written(&pkt, &written)
-            ||  written != sizeof(simple1)
-            ||  memcmp(buf->data, &simple1, written) != 0
-            ||  len != 1) {
-        testfail("test_WPACKET_set_packet_len():2 failed\n", &pkt);
-        return 0;
-    }
-
-    return 1;
-}
-
 static int test_WPACKET_set_max_size(void)
 {
     WPACKET pkt;
     size_t written;
-    unsigned char len;
 
     if (       !WPACKET_init(&pkt, buf)
                 /*
@@ -148,16 +113,26 @@ static int test_WPACKET_set_max_size(void)
             || !WPACKET_set_max_size(&pkt, SIZE_MAX -1)
                 /* And setting it bigger again should be ok */
             || !WPACKET_set_max_size(&pkt, SIZE_MAX)
-            || !WPACKET_set_packet_len(&pkt, &len, 1)
+            || !WPACKET_finish(&pkt)) {
+        testfail("test_WPACKET_set_max_size():1 failed\n", &pkt);
+        return 0; 
+    }
+
+    if (       !WPACKET_init_len(&pkt, buf, 1)
+                /*
+                 * Should fail because we already consumed 1 byte with the
+                 * length
+                 */
+            ||  WPACKET_set_max_size(&pkt, 0)
                 /*
                  * Max size can't be bigger than biggest that will fit in
                  * lenbytes
                  */
             ||  WPACKET_set_max_size(&pkt, 0x0101)
                 /* It can be the same as the maximum possible size */
-            || !WPACKET_set_max_size(&pkt, 0xff)
+            || !WPACKET_set_max_size(&pkt, 0x0100)
                 /* Or it can be less */
-            || !WPACKET_set_max_size(&pkt, 0x00)
+            || !WPACKET_set_max_size(&pkt, 0x01)
                 /*
                  * Should fail because packet is already filled
                  */
@@ -165,34 +140,13 @@ static int test_WPACKET_set_max_size(void)
                 /*
                  * You can't put in more bytes than max size
                  */
-            || !WPACKET_set_max_size(&pkt, 0x01)
-            || !WPACKET_put_bytes(&pkt, 0xff, 1)
-            ||  WPACKET_put_bytes(&pkt, 0xff, 1)
-            || !WPACKET_finish(&pkt)
-            || !WPACKET_get_total_written(&pkt, &written)
-            ||  written != sizeof(simple1)
-            ||  memcmp(buf->data, &simple1, written) != 0
-            ||  len != 1) {
-        testfail("test_WPACKET_set_max_size():1 failed\n", &pkt);
-        return 0;
-    }
-
-    if (       !WPACKET_init_len(&pkt, buf, 1)
-                /*
-                 * Should fail because we already consumed 1 byte with the
-                 * length
-                 */
-            ||  WPACKET_set_max_size(&pkt, 0)
-            || !WPACKET_set_max_size(&pkt, 1)
-            ||  WPACKET_put_bytes(&pkt, 0xff, 1)
-            || !WPACKET_set_max_size(&pkt, 2)
+            || !WPACKET_set_max_size(&pkt, 0x02)
             || !WPACKET_put_bytes(&pkt, 0xff, 1)
             ||  WPACKET_put_bytes(&pkt, 0xff, 1)
             || !WPACKET_finish(&pkt)
             || !WPACKET_get_total_written(&pkt, &written)
             ||  written != sizeof(simple2)
-            ||  memcmp(buf->data, &simple2, written) != 0
-            ||  len != 1) {
+            ||  memcmp(buf->data, &simple2, written) != 0) {
         testfail("test_WPACKET_set_max_size():2 failed\n", &pkt);
         return 0;
     }
@@ -283,7 +237,7 @@ static int test_WPACKET_set_flags(void)
 
     /* Set packet to be non-zero length */
     if (       !WPACKET_init(&pkt, buf)
-            || !WPACKET_set_flags(&pkt, OPENSSL_WPACKET_FLAGS_NON_ZERO_LENGTH)
+            || !WPACKET_set_flags(&pkt, WPACKET_FLAGS_NON_ZERO_LENGTH)
                 /* Should fail because of zero length */
             ||  WPACKET_finish(&pkt)
             || !WPACKET_put_bytes(&pkt, 0xff, 1)
@@ -298,7 +252,7 @@ static int test_WPACKET_set_flags(void)
     /* Repeat above test in a sub-packet */
     if (       !WPACKET_init(&pkt, buf)
             || !WPACKET_start_sub_packet(&pkt)
-            || !WPACKET_set_flags(&pkt, OPENSSL_WPACKET_FLAGS_NON_ZERO_LENGTH)
+            || !WPACKET_set_flags(&pkt, WPACKET_FLAGS_NON_ZERO_LENGTH)
                 /* Should fail because of zero length */
             ||  WPACKET_close(&pkt)
             || !WPACKET_put_bytes(&pkt, 0xff, 1)
@@ -313,8 +267,7 @@ static int test_WPACKET_set_flags(void)
 
     /* Set packet to abandon non-zero length */
     if (       !WPACKET_init_len(&pkt, buf, 1)
-            || !WPACKET_set_flags(&pkt,
-                                  OPENSSL_WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH)
+            || !WPACKET_set_flags(&pkt, WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH)
             || !WPACKET_finish(&pkt)
             || !WPACKET_get_total_written(&pkt, &written)
             ||  written != 0) {
@@ -325,8 +278,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_set_flags(&pkt,
-                                  OPENSSL_WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH)
+            || !WPACKET_set_flags(&pkt, WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH)
             || !WPACKET_close(&pkt)
             || !WPACKET_finish(&pkt)
             || !WPACKET_get_total_written(&pkt, &written)
@@ -339,8 +291,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_set_flags(&pkt,
-                                  OPENSSL_WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH)
+            || !WPACKET_set_flags(&pkt, WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH)
             || !WPACKET_put_bytes(&pkt, 0xff, 1)
             || !WPACKET_close(&pkt)
             || !WPACKET_finish(&pkt)
@@ -420,7 +371,6 @@ int main(int argc, char *argv[])
     buf = BUF_MEM_new();
     if (buf != NULL) {
         ADD_TEST(test_WPACKET_init);
-        ADD_TEST(test_WPACKET_set_packet_len);
         ADD_TEST(test_WPACKET_set_max_size);
         ADD_TEST(test_WPACKET_start_sub_packet);
         ADD_TEST(test_WPACKET_set_flags);