#538 chsnprintf sometimes doesn't terminate str

[Colin D Bennett]

If the chsnprintf() function fills up the result buffer str (storing size characters into it), it does not NUL-terminate str.

This is inconsistent with the contract of the standard snprintf() and makes it difficult to write bug-free programs using chsnprintf(). The expected behavior is to always store a zero byte in str (as long as size is not 0). If an attempt is made to print size characters into str, the last character should be omitted and the zero byte stored instead.

Test case

// The test tries to print "1234" to the buffer.
// But the buffer has a capacity of only 4 bytes, so in order to
// satisfy the NUL termination requirement, the final '4' must be omitted.
// We expect the result to be "123\0".
test_chsnprintf();
test_std_snprintf();
printf("         expected: 0x%02X 0x%02X 0x%02X 0x%02X; returned %d\n",
       '1', '2', '3', '\0', 3);


static void test_chsnprintf(void)
{
    printf("\n-- chsnprintf --\n");
    // Verify that chsnprintf ALWAYS zero-terminates the string.
    char buf[4] = { 'A', 'B', 'C', '\0' };
    int n = chsnprintf(buf, sizeof(buf), "%d", 1234);
    printf("chsnprintf result: 0x%02X 0x%02X 0x%02X 0x%02X; returned %d\n",
        buf[0], buf[1], buf[2], buf[3], n);
}

static void test_std_snprintf(void)
{
    printf("\n-- C Standard Library snprintf --\n");
    // snprintf will ALWAYS zero-terminate the string.
    char buf[4] = { 'A', 'B', 'C', '\0' };
    int n = snprintf(buf, sizeof(buf), "%d", 1234);
    printf("snprintf result:   0x%02X 0x%02X 0x%02X 0x%02X; returned %d\n",
        buf[0], buf[1], buf[2], buf[3], n);
}

Output:

-- chsnprintf --
chsnprintf result: 0x31 0x32 0x33 0x34; returned 3

-- C Standard Library snprintf --
snprintf result:   0x31 0x32 0x33 0x00; returned 4
         expected: 0x31 0x32 0x33 0x00; returned 3

NOTE that chsnprintf did not store a 0x00 as the last byte, so the output string is unterminated.

This was tested against chprintf.c from as of SVN trunk r7336.

[Giovanni Di Sirio]

Hi,

Thanks for finding, I committed a tentative fix in the repository.

Giovanni

[Colin D Bennett]

Fix on 2014-10-04 in trunk rev 7358 is a partial fix. But there are still two problems:

1. If size==0, the the buffer is overrun, essentially unbounded.
2. Return value of chsnprintf() doesn't match snprintf() spec.

I have a patch that should resolve these issues fully.

To address point 2, it changes chvprintf to return int instead of void. Which matches standard vprintf and allows chsnprintf() to detect when the number of characters that were attempted to be written was too much for the buffer, and return a result accordingly. Then chsnprintf() can use this return value.

An alternative solution to point 2 is would be to change the MemoryStream to keep track of the number of attempted put operations.

[Giovanni Di Sirio]

Thanks for the patch, committed.

Giovanni