Does this code from “The C Programming Language 2nd Edition” contain a bug?

Question

#include <stdio.h>
#include <stdlib.h>
#include <stdarg.h>
void minprintf(char *fmt, ...)
{
    va_list ap;
    char *p, *sval;
    int ival;
    double dval;

    va_start(ap, fmt);
    for (p = fmt; *p; p++) {
        if (*p != '%') {
            putchar(*p);
            continue;
        }
        switch (*p++) {
        case 'd':
            ival = va_arg(ap, int);
            printf("%d", ival);
            break;
        case 'f':
            dval = va_arg(ap, double);
            printf("%f", dval);
            break;
        case 's':
            for (sval = va_arg(ap, char *); *sval; sval++)
                putchar(*sval);
            break;
        default:
            putchar(*p);
            break;
        }
    }
    va_end(ap);
}

int main(void)
{
    minprintf("aaaaaaa%\0dddd");
    return 0;
}

This code is from the C programming language second edition 7.3 Variable-length Argument Lists

Normally this program should output aaaaaaa and stops but instead it prints aaaaaaa dddd. http://ideone.com/d3Akk

Is that really a bug.

Thank you.

Answer 1

The thing is you ignore the null terminator if it is preceded by % (in the switch statement). That may or may not be a bug, but is certainly nonstandard behavior for C functions. However, in your case, it doesn't cause any undefined behavior and pretty much does what it says.

Answer 2

The function called with a format string like "aaa%" will cause UB, breaking the principle of least surprise. This is a bug in my book.

Answer 3

Your problem being that because of the for condition *p you'd expect it to stop at the first NULL, only it doesn't?

So your question, is: "why does it not stop at the first NULL?". Answer: because of the post-increment in the switch() statement. It evaluates the switch-block first, then increments the pointer. So what happens in your specific case is that when the function sees the percent sign it drops into the switch statement. Because NULL is not a valid format specifier, the switch block defaults to outputting it. Then, because of the post-increment the pointer is moved ahead one character, which is a d. Therefore, *p works out as d, which is not 0, therefore the condition in the for loop is defined to be true.

EDIT: There is a bug in there, IMO, but it is not actually this one: it is the fact that wrong format specifiers are silently discarded by the default construct. Additionally, there may be an edge case if you'd do something like minprintf("whoopsie%"); where the for loop will attempt to iterate past the end of the string!