HiveBrain v1.2.0
Get Started
← Back to all entries
snippetcppMinor

Compile-time printf format checking

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
formatcompilecheckingtimeprintf

Problem

Compile time checking of printf-like format strings

Inspired by this open ticket on boost, this seeks to complete the work there

Given a printf-style format string and associated arguments, a static_assert is performed on whether the format string and arguments are valid.

I'm particularly interested in:

  • Have I covered all possible format strings?



  • Am I doing this in the most efficient way?



Code:

Working example on ideone

```
#include
#include
#include

template
constexpr bool checkValidFormats(const char (&fmt)[N], size_t n, char c)
{
return n >= N ?
false
: fmt[n] == c ?
true
: checkValidFormats(fmt, n + 1, c);
}

template
struct FormatSupportedType;

#define SUPPORTED_TYPE(T, Fmts) \
template<> \
struct FormatSupportedType \
{ \
constexpr static bool supports(char c) \
{ return checkValidFormats(Fmts, 0, c) \
? true : throw std::logic_error("invalid fmt for type"); } \
}

SUPPORTED_TYPE(char, "c");
SUPPORTED_TYPE(int, "d*");
SUPPORTED_TYPE(unsigned, "u*");
SUPPORTED_TYPE(char*, "s");
SUPPORTED_TYPE(const char*, "s");
SUPPORTED_TYPE(std::string, "s");
SUPPORTED_TYPE(boost::string_ref, "s");
SUPPORTED_TYPE(double, "f");
SUPPORTED_TYPE(float, "f");

/////////////////

constexpr bool isDigit(char c)
{
return c >= '0' && c
constexpr size_t nextNonModifier(const char (&fmt)[N], std::size_t n)
{
return
n >= N ?
throw std::logic_error("invalid format string")
: isModifier(fmt[n]) ?
nextNonModifier(fmt, n + 1)
: n;
}

////////////////////

template
constexpr bool checkFormatHelper(const char (&fmt)[N], std::size_t n);
template
constexpr bool checkFormatHelper(const char (&fmt)[N], std::size_t n, const T& arg, const Ts&... args);

////////////////////

template
constexpr auto checkWidthAndPrecision(const char (&fmt)[N], std::size_t n, const T1& /width/

Solution

Overall a nice start.

But it is by no means complete.
The ## symbol joins the prev and the next token.

static_assert(checkFormat(fmt, ##__VA_ARGS__), "Format is incorrect"); \
    boost::format f(fmt); \
    add(f, ##__VA_ARGS__); \


So I am not sure what you are doing with it here. I would remove it completely.

@glampert pointed out that the ## used with varargs on GNU is a special extension that effectively drops the proceeding comma if the argument list is empty.
Using ternary operator as an if/then/else

I think your use of the ternary operator makes the code less readable.

return
    n>= N ?
        true
    : fmt[n] != '%' ?
        checkFormatHelper(fmt, n + 1)
    : fmt[n + 1] == '%' ?
        checkFormatHelper(fmt, n + 2)
    : false;


Is that as readable as:

if (n>= N) {
    return true;
}
else if (fmt[n] != '%') {
    return checkFormatHelper(fmt, n + 1);
}
else if (fmt[n + 1] == '%')
    return checkFormatHelper(fmt, n + 2);
}


I suppose its a debatable one that. But your main loop becomes a bit more obtuse. I understand you are probably doing it because of the constexpr requirements.
Format Specifier:

Also the format specifier is a bit more complex than this:

: fmt[n + 1] == '%' ?
            checkFormatHelper(fmt, n + 2)


The format specifier is generalized to:

%[][][.][]

        flags     := [-+ #0]*            // Zero or more 
        width     := |'*'        // A number or a '*'
        precision := |'*'
        length    := hh|h|l|ll|j|z|t|L
        specifier := d|i|u|o|x|X|f|F|e|E|g|G|a|A|c|s|p|n|%


So you need to use something more like the code in your main helper function (looks like a case for re-use).
Length

You support the long long.

// long-long modifier
    : (fmt[n + 1] == 'l' && fmt[n + 2] == 'l') ?
        FormatSupportedType::type >::supports(fmt[n + 3]) &&
        checkFormatHelper(fmt, n + 4, args...)


But that is only one of several length specifiers.

Also you are not checking to see if the other optional parts of the format specifier is there you just assume it comes directly after the ll. You need your checkValidFormats() to be slightly more sophisticated.
Width/Precision

// width & precision modifier
    : (fmt[n + 1] == '*' && fmt[n + 2] == '.' && fmt[n + 3] == '*') ?
        checkWidthAndPrecision(fmt, n + 4, arg, args...)

    // width or precision modifier
    : ((fmt[n + 1] == '.' && fmt[n + 2] == '*') || (fmt[n + 1] == '*')) ?
        checkWidthOrPrecision(fmt, (fmt[n + 1] == '.' ? n + 3 : n + 2), arg, args...)


Note these values can also be numbers. Which means you should be skipping over the number characters.

But I think your code is incorrect here. Because if the width or precision specifier is * then you should be expecting another runtime argument. Which means I would expect you to be checking the type of the current argument (which should be some form of integer).
Recomendation

I would split the checkFormatHelper() into multiple functions that check the different parts of the format specifier.

```
template
constexpr bool checkFormatHelper(const char (&fmt)[N], std::size_t n, const T& arg, const Ts&... args)
{
return
n >= N ?
throw std::logic_error("too many arguments for provided format string")

: fmt[n] != '%' ?
checkFormatHelper(fmt, n + 1, arg, args...)

// literal percent character
: (fmt[n + 1] == '%') ?
checkFormatHelper(fmt, n + 2, arg, args...)

// Otherwise we have a format specifier.
// Check each part of the specifier
: checkFormatHelperFlags(fmt, n + 1, arg, args...);
}

template
constexpr bool checkFormatHelperFlags(const char (&fmt)[N], std::size_t n, const T& arg, const Ts&... args)
{
return
fmt[n] == '-' ? checkFormatHelperFlags(fmt, n+1, arg, args...)
: fmt[n] == '+' ? checkFormatHelperFlags(fmt, n+1, arg, args...)
: fmt[n] == ' ' ? checkFormatHelperFlags(fmt, n+1, arg, args...)
: fmt[n] == '#' ? checkFormatHelperFlags(fmt, n+1, arg, args...)
: fmt[n] == '0' ? checkFormatHelperFlags(fmt, n+1, arg, args...)
: checkFormatHelperWidth(fmt, n, arg, args...);
}
template
constexpr bool checkFormatHelperWidth(const char (&fmt)[N], std::size_t n, const T& arg, const Ts&... args)
{
return
isDigit(fmt[n]) ?
? checkFormatHelperPrecision(fmt, removeDigits(fmt,n) , arg, args...);
: fmt[n] == '*'
? FormatSupportedType::type>::supports("i") && checkFormatHelperPrecision(fmt, n+1, args...)
: checkFormatHelperPrecision(fmt, n, arg, args...);
}
template
constexpr bool checkFormatHelperPrecision(const char (&fmt)[N], std::size_t n, const T& arg, const Ts&... args)
{
return
fmt[n] == '.'
? isDigit(fmt[n])
? checkFormatHelperLength(fmt, removeDigits(fmt,n) , arg, args...);
: fmt[n] == '*'
? FormatS

Code Snippets

static_assert(checkFormat(fmt, ##__VA_ARGS__), "Format is incorrect"); \
    boost::format f(fmt); \
    add(f, ##__VA_ARGS__); \
return
    n>= N ?
        true
    : fmt[n] != '%' ?
        checkFormatHelper(fmt, n + 1)
    : fmt[n + 1] == '%' ?
        checkFormatHelper(fmt, n + 2)
    : false;
if (n>= N) {
    return true;
}
else if (fmt[n] != '%') {
    return checkFormatHelper(fmt, n + 1);
}
else if (fmt[n + 1] == '%')
    return checkFormatHelper(fmt, n + 2);
}
: fmt[n + 1] == '%' ?
            checkFormatHelper(fmt, n + 2)
%[<flags>][<width>][.<precision>][<length>]<specifier>

        flags     := [-+ #0]*            // Zero or more 
        width     := <number>|'*'        // A number or a '*'
        precision := <number>|'*'
        length    := hh|h|l|ll|j|z|t|L
        specifier := d|i|u|o|x|X|f|F|e|E|g|G|a|A|c|s|p|n|%

Context

StackExchange Code Review Q#84768, answer score: 2

Revisions (0)

No revisions yet.