11

We are using a third-party C library which provides a printf()-style log function,

void log(const char *format, ...);

For reasons that aren't worth going in to, we need to limit the rate at which messages get logged, something along the lines of

void rate_limited_log(const char* format, ...)
{
    if (<not too fast>) {
         log(format, ...);
    }
}

Fortunately the authors of the C library knew what they were doing, and provided

void logv(const char* format, va_list ap);

so writing the above function is a relatively simple matter. Unfortunately however variadic functions don't play well with inlining, and so I came up with a second solution:

template <typename... T>
void rate_limited_log(const char* format, T&&... args)
{
    if (<not too fast>) {
        log(format, std::forward<T>(args)...);
    }
}

This works perfectly and inlines the rate limiting condition as we'd like. But I have a couple of questions about it:

  • Is expanding a parameter pack into a C-style variadic function call like this a legal, well-defined thing to do in C++11, or have we just got lucky that it works?

  • Are the && and std::forward actually necessary here, given we're calling a C function? It seems to work just as well if I use const T&, or even just T by value, with or without std::forward.

Community
  • 1
  • 1
Tristan Brindle
  • 16,281
  • 4
  • 39
  • 82

2 Answers2

9

Expanding parameter packs into varargs is valid.

And there is no harm in forwarding when you want to forward. But taking by const& also communicates something useful.

The values passed to ... will experience "default argument promotions". See http://en.cppreference.com/w/cpp/language/variadic_arguments

None of these care about references.

You can improve your code. You can check that the Ts... are valid types to pass to the printing routine, both by "kind of type" and by actually parsing the formatting string and confirming number (and sometimes type) of arguments. If this fails, you can log an error message instead of crashing.

Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524
3

I'm not sure that inlining matters for a logging function. Indeed, many C compilers don't inline variadic functions. However, you could make it a macro

#define RATE_LIMITED_LOG(Fmt,...) do { \
   if (not_too_fast())      \
     log(Fmt,__VA_ARGS__);  \
} while(0)

Especially for logging functions, making them a macro is good, because they could use __LINE__ and __FILE__ like this

#define RATE_LIMITED_LOG_AT2(Fil,Lin,Fmt,...) do {
   if (not_too_fast())
     log("%s:%d " Fmt, Fil, (Lin), __VA_ARGS__);
} while(0)
#define RATE_LIMITED_LOG_AT(Fil,Lin,Fmt,...) \
   RATE_LIMITED_LOG_AT2(Fil,Lin,Fmt,__VA_ARGS__)
#define RATE_LIMITED_LOG(Fmt,...) \
   RATE_LIMITED_LOG_AT(__FILE__,__LINE__,Fmt,__VA_ARGS__)

Notice the string literal catenation of "%s:%d " with the real Fmt which is supposed to always be a literal formal string. Typical use would be RATE_LIMITED_LOG("x=%d y=%d", x, y);...

I admit it is low tech and very C like (not using any C++11 gadget), but it works quite well in practice. In practice I'm using a software specific prefix to the macro (like my MOM_FATAPRINTF in monimelt.h, which is in C not C++).

Basile Starynkevitch
  • 223,805
  • 18
  • 296
  • 547
  • 1
    Myself, I'd use an outer macro that injects `__FILE__` and `__LINE__` into an inner function, simply because debugging macro generated code is often painful. Do what needs to be done (and sometimes things like string concatenation, which can be faster) in a macro, and everything else in plain old code (PS: you need some more `\`s above) – Yakk - Adam Nevraumont Aug 27 '14 at 15:24