Sammy
Sammy

Reputation: 277

snprintf: Same code - different errors/warnings on different g++ compilers

I know this question may have been asked before but hasn't answered the query I am posting below. I am trying to figure out why exactly the code below gives the error (and in other version of GCC, a warning) and esp. how to resolve this? I am an experienced C++ programmer, but sometimes you do need to seek for help. Any help is appreciated.

[ERROR]: cannot pass objects of non-trivially-copyable type ‘class std::basic_string’ through

/// USE THE MACRO BELOW TO LOG A FORMATTED STRING (usage is exactly like printf(...) [ SEE HEADER FILE - TEMPLATE FUNCTION user_log_fmt(...) ]
#define LOG_TRACE_FMT(...)  Logger::getInstance()->user_log_fmt(LOG_LEVEL_TRACE,   __PRETTY_FUNCTION__, __FUNCTION__, __VA_ARGS__)

void main()
{   
    std::string linkName = getLinkName();

    // THIS IS THE CALL THAT LEADS TO THE ERROR AT COMPILE-TIME
    LOG_TRACE_FMT("\nLink-name: %s, Sent packet: SYS: %d, COMP: %d, LEN: %d, MSG ID: %d\n", linkName, msg->sysid, msg->compid, msg->len, msg->msgid);
}
///
/// Part of logger class (combines strings with argument for formatting) 
///
template <typename ... Args>
void user_log_fmt(LOG_LEVEL level,  std::string pretty_func, std::string func_name, std::string str_format, Args ... args)
{
    if (m_LogLevel < level)
        return;

    std::string formatted_data = utils::Utils::string_format(str_format, args...);
    // Do something with the formatted string.. 
}

///
/// Part of Utils library. This method does a printf type formatting.
///
template<typename ... Args>
static std::string string_format( const std::string& format, Args ... args )
{
    size_t size = snprintf( nullptr, 0, format.c_str(), args ... ) + 1; // Extra space for '\0'
    std::unique_ptr<char[]> buf( new char[ size ] );
    snprintf( buf.get(), size, format.c_str(), args ... );
    return std::string( buf.get(), buf.get() + size - 1 ); // We don't want the '\0' inside
}

The error is with snprintf statement above.

WARNING on g++ (Ubuntu 5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609:

warning: format not a string literal and no format arguments [-Wformat-security] size_t size = snprintf( nullptr, 0, format.c_str(), args ... ) + 1;' ^

ERROR on g++ (Raspbian 4.9.2-10) 4.9.2

error: cannot pass objects of non-trivially-copyable type ‘class std::basic_string’ through ‘...’ size_t size = snprintf( nullptr, 0, format.c_str(), args ... ) + 1;

Note: I wish to understand and resolve this but without providing a compiler flag setting. I understand this may have something to do with C compatibility with C++.

Thanks!

Upvotes: 0

Views: 1009

Answers (2)

I don't know what is causing your problems, but I don't see what benefit you gain from using a template function. You can just use good-old-fashioned varargs for this.

I was going to say, that the golden rule for writing a varargs function is to write one which takes a va_list. Your edit makes the reason for that very clear - you usually end up calling the varags function from another varargs function!

#include <stdarg.h>  // varargs support

static std::string string_vformat( const std::string& format, va_list args)
{
    va_list args_copy;
    va_copy(args,args_copy);  // Copy args before we use it.
                              // +1 is extra space for '\0'
    const size_t size = vsnprintf( nullptr, 0, format.c_str(), args_copy ) + 1; 
    va_end(args_copy);        // Release args_copy.
    std::unique_ptr<char[]> buf( new char[ size ] );
    vsnprintf( buf.get(), size, format.c_str(), args );
    // Caller must va_end(args);
           // We don't want the '\0' inside
    return std::string( buf.get(), buf.get() + size - 1 );
}

static std::string string_format( const std::string& format, ... )
{
    va_list args;
    va_start(format, args);
    const auto result = string_vformat(format, args);
    va_end(args);
    return result;
}

void user_log_fmt(LOG_LEVEL level,  std::string pretty_func, std::string func_name,
                                    std::string str_format, ... )
{
    if (m_LogLevel < level)
        return;

    va_list args;
    va_start(format, args);
    std::string formatted_data = utils::Utils::string_vformat(str_format, args);
    // Do something with the formatted string.. 
}

A couple of asides:

  • I would allocate a writeable string with room for the '\0' and write into the buffer for that and then shrink the buffer afterwards to lose the terminator. Strings are contiguous, so this is safe, and saving a dynamic allocation makes the cost of the extra byte in the final result very worthwhile.
  • I would take all the arguments of user_log_fmt as const reference to std::string (but that's just because pass-by-value always makes me twitch).

Finally, as you appear to be using GCC, you can write:

#ifdef __GNUC__
    #define PRINTFLIKE(n)  __attribute__(format,printf,(n),(n+1))
#else
    #define PRINTFLIKE(n)
#endif

and then you can declare:

 void user_log_fmt(LOG_LEVEL level, std::string pretty_func, std::string func_name,
                  std::string str_format, ... ) PRINTFLIKE(4)

and

 std::string string_format( const std::string& format, ... ) PRINTFLIKE(1)

Then GCC will issue you with a nice error if you do something like:

string_format( "Two things %d %d", only_one_thing );

(It will also complain if the format is the wrong type.

Upvotes: 1

Sammy
Sammy

Reputation: 277

I found an answer to this ambiguity. Thanks @PaulMcKenzie for pointing out. Here was the solution I saw worked with any compiler flag such as "-Wno-format-security". No more error on Pi or Error on Ubuntu g++.

void main()
{   
    std::string linkName = getLinkName();

    // THIS IS THE CALL THAT LEADS TO THE ERROR AT COMPILE-TIME
    LOG_TRACE_FMT("\nLink-name: %s, Sent packet: SYS: %d, COMP: %d, LEN: %d, MSG ID: %d\n", linkName.c_str(), msg->sysid, msg->compid, msg->len, msg->msgid);
}

The catch was doing linkName.c_str() (C-Style string) instead of passing the std::string as argument. This was tricky!

Upvotes: 1

Related Questions