Clarify bin_to_readable code

I got a bit of a scare when I first saw this code. Turns out that
the way it's used inlen==outlen and that's why it works.

diff --git a/ext/session/session.c b/ext/session/session.c
index a2642807ad44d23e80b00ef09d511c496e661632..bfd1c9b566d41a4f98d8b0cde6dcc2fc3e4eb38b 100644 (file)
--- a/ext/session/session.c
+++ b/ext/session/session.c
@@ -268,12 +268,10 @@ static int php_session_decode(zend_string *data) /* {{{ */
 
 static char hexconvtab[] = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ,-";
 
-/* returns a pointer to the byte after the last valid character in out */
-static size_t bin_to_readable(unsigned char *in, size_t inlen, char *out, char nbits) /* {{{ */
+static void bin_to_readable(unsigned char *in, size_t inlen, char *out, size_t outlen, char nbits) /* {{{ */
 {
        unsigned char *p, *q;
        unsigned short w;
-       size_t len = inlen;
        int mask;
        int have;
 
@@ -284,16 +282,15 @@ static size_t bin_to_readable(unsigned char *in, size_t inlen, char *out, char n
        have = 0;
        mask = (1 << nbits) - 1;
 
-       while (inlen--) {
+       while (outlen--) {
                if (have < nbits) {
                        if (p < q) {
                                w |= *p++ << have;
                                have += 8;
                        } else {
-                               /* consumed everything? */
-                               if (have == 0) break;
-                               /* No? We need a final round */
-                               have = nbits;
+                               /* Should never happen. Input must be large enough. */
+                               ZEND_ASSERT(0);
+                               break;
                        }
                }
 
@@ -304,7 +301,6 @@ static size_t bin_to_readable(unsigned char *in, size_t inlen, char *out, char n
        }
 
        *out = '\0';
-       return len;
 }
 /* }}} */
 
@@ -315,13 +311,18 @@ PHPAPI zend_string *php_session_create_id(PS_CREATE_SID_ARGS) /* {{{ */
        unsigned char rbuf[PS_MAX_SID_LENGTH + PS_EXTRA_RAND_BYTES];
        zend_string *outid;
 
+       /* It would be enough to read ceil(sid_length * sid_bits_per_character / 8) bytes here.
+        * We read sid_length bytes instead for simplicity. */
        /* Read additional PS_EXTRA_RAND_BYTES just in case CSPRNG is not safe enough */
        if (php_random_bytes_throw(rbuf, PS(sid_length) + PS_EXTRA_RAND_BYTES) == FAILURE) {
                return NULL;
        }
 
        outid = zend_string_alloc(PS(sid_length), 0);
-       ZSTR_LEN(outid) = bin_to_readable(rbuf, PS(sid_length), ZSTR_VAL(outid), (char)PS(sid_bits_per_character));
+       bin_to_readable(
+               rbuf, PS(sid_length),
+               ZSTR_VAL(outid), ZSTR_LEN(outid),
+               (char)PS(sid_bits_per_character));
 
        return outid;
 }