Encapsulating snprintf to simplify usage: sbprintf & swnprintf

Question

After writing swnprintfand sbprintf in C (Encapsulating snprintf to avoid repetition of sizeof), I've written a C++ version of them:

---

swnprintf:

Similar to snprintf. Properties: (properties not mentioned shall be the same as snprintf)

• It writes the actual value of characters written (excluding the NUL terminator) through a pointer, instead of returning it.
• The user can explicitly discard that value by passing NULL.
• If a NUL character has been written in the middle of the string (eg.: swnprintf(buf, &w, sizeof(buf), "%c%c%c", 'a', 0, 'b');), the last NUL written is not counted, but any other characters until that one, including NULs are counted. In this example, w == 3 after the function call.
• If the string is truncated, the value written is the actual number of characters written, and not the value that would have been written had n been sufficiently large.
• If there's an error in the internal call to snprintf, the resulting string is unreliable (written is also unreliable) and the return value is negative. If there's truncation, the string is valid and the return value is positive. If there's no error, the return value is 0.
• errno is set on any error. Truncation: ENOMEM. The absolute value of the return value is the same as errno.

Usage:

• Example where the user doesn't care about truncation:

char        buf[BUFSIZ];
ptrdiff_t   tmp;
ptrdiff_t   len;

if (alx::swnprintf(buf, &tmp, sizeof(buf), "text, num=%i", 7) < 0)
        goto err;
len = tmp;
if (alx::swnprintf(&buf[len], &tmp, sizeof(buf) - len, "2nd part") < 0)
        goto err;
len += tmp;

• Example where the user cares about truncation and doesn't about len:

char        cmd[_POSIX_ARG_MAX];
ptrdiff_t   tmp;

if (alx::swnprintf(cmd, &tmp, sizeof(cmd), "%s ", "cat"))
        goto err;
if (alx::swnprintf(&cmd[tmp], NULL, sizeof(cmd) - tmp, " %s ", "main.c"))
        goto err;
system(cmd);

---

sbprintf:

This is a higher abstraction than swnprintf. It is designed to only accept arrays as input. It is safer because it calculates internally the size of the buffer, so the user has less chance of writing buggy code. The down side is that it is less flexible (it can only write at the beginning of a buffer). Apart from that, the behaviour is the same as in swnprintf.

Properties: (properties not mentioned shall be the same as swnprintf)

• It shall only compile if the string is a char [] and not a char *.
• It is impossible to write past the buffer, because the user doesn't input its size.

Usage:

• Example where the user doesn't care about truncation:

char        buf[BUFSIZ];
ptrdiff_t   len;

if (alx_sbprintf(buf, &len, "text, num=%i", 7) < 0)
        goto err;

• Example where the user cares about truncation and doesn't about len:

char    cmd[_POSIX_ARG_MAX];

if (alx_sbprintf(cmd, NULL, "%s %s", "cat", "main.c"))
        goto err;
system(cmd);

---

---

Implementation:

swnprintf:

swnprintf.hpp:

#ifndef ALX_STDIO_PRINTF_SWNPRINTF_HPP
#define ALX_STDIO_PRINTF_SWNPRINTF_HPP


#include <cstdarg>
#include <cstddef>


namespace alx {


int swnprintf(char *__restrict__ str, ptrdiff_t *__restrict__ written,
              ptrdiff_t nmemb, const char *__restrict__ format, ...);


}       /* namespace alx */


#endif      /* libalx/base/stdio/printf/swnprintf.hpp */

swnprintf.cpp:

#include "libalx/base/stdio/printf/swnprintf.hpp"

#include <cerrno>
#include <cstdarg>
#include <cstddef>
#include <cstdio>


namespace alx {


int swnprintf(char *__restrict__ str, ptrdiff_t *__restrict__ written,
              ptrdiff_t nmemb, const char *__restrict__ format, ...)
{
        va_list ap;
        int     len;

        if (nmemb < 0)
                goto neg;

        va_start(ap, format);
        len     = vsnprintf(str, nmemb, format, ap);
        va_end(ap);

        if (written != NULL)
                *written = len;

        if (len < 0)
                goto err;
        if ((unsigned)len >= nmemb)
                goto trunc;

        return  0;
err:
        return  -errno;
trunc:
        if (written)
                *written = nmemb - 1;
        errno   = ENOMEM;
        return  ENOMEM;
neg:
        errno   = EOVERFLOW;
        return  -EOVERFLOW;
}


}       /* namespace alx */

---

sbprintf:

sbprintf.hpp:

#ifndef ALX_STDIO_PRINTF_SBPRINTF_HPP
#define ALX_STDIO_PRINTF_SBPRINTF_HPP


#include "libalx/base/assert/assert.hpp"
#include "libalx/base/stdio/printf/swnprintf.hpp"


namespace alx {


/*
 * int  alx_sbprintf(char buff[__restrict__], ptrdiff_t *__restrict__ written,
 *                   const char *__restrict__ fmt, ...);
 */
#define alx_sbprintf(buff, written, fmt, ...)   (                       \
{                                                                       \
                                                                        \
        alx_static_assert_char_array(buff);                             \
        alx::swnprintf(buff, written, sizeof(buff), fmt, ##__VA_ARGS__);\
}                                                                       \
)


}       /* namespace alx */


#endif      /* libalx/base/stdio/printf/sbprintf.hpp */

---

Macros used by the code above:

assert.hpp:

#ifndef ALX_ASSERT_ASSERT_HPP
#define ALX_ASSERT_ASSERT_HPP


#include <cassert>

#include <type_traits>


namespace alx {


#define alx_static_assert_array(a)  do                                  \
{                                                                       \
                                                                        \
        static_assert(std::is_array <typeof(a)>::value, "Not a `[]`!"); \
} while (0)

#define alx_static_assert_char_array(a)     do                          \
{                                                                       \
                                                                        \
        alx_static_assert_array(a);                                     \
        static_assert(std::is_same <char, typeof((a)[0])>::value,       \
                                                "Not a `char[]`!");     \
} while (0)


}       /* namespace alx */


#endif      /* libalx/base/assert/assert.hpp */

Is there anything that can be improved in these two functions? I'm starting with C++ (I'm used to C), so I don't know if there are more efficient/safe methods to do this.

Improvements in the usage (interface) are very welcome.

Is ENOMEM adecuate when the string has been truncated? My other candidate was EOVERFLOW, but it was already being used by snprintf so I thought it might be better to choose a different one.

---

Edit

The reasons behind so many gotos are:

• Unconditional statements are easier to understand and follow: The error-free path has the minimum conditional statements possible, and they are as short as possible, and therefore, the main code is more compact vertically. When you only want to know what the function does, you can read the first part of the body, and assume the gotos will do some cleanup job. Given that eye movement is easier horizontally than vertically, that makes the code easier for the eyes.

• Nesting is reduced: Those ifs after the goto would be nested to the if that reports the error. With 8-char tabs, that lets you have more characters per line. Also less eye movement (in this case horizontal).

• Use of braces is reduced: That's a lot of almost-empty lines saved, and therefore, again less vertical movement for the eyes.

• Errors by not updating individual exit points when making modifications are prevented: I said that when you want to know what the function does, you read the first part; when you want to know how the function handles error conditions, you read the last part. All error handling is together, so again, less vertical movement. When reading this part you can know where the gotos come from by their labels (I hope they are meaningful enough), and focus only on the error handling itself.

• Although it can help the optimizer to remove duplicated code some times, that's a secondary reason for me; and sometimes I've seen it the other way (less efficient code). When the function is not critical, I just don't care; when it is, I do whatever is best.

Answer 1

<cstddef> defines std::ptrdiff_t - the implementation is allowed to also define it in the global namespace, but it's not required to - so always qualify the type.

I was expecting to see sbprintf() defined as a template, rather than as a macro (as an aside, macros work at the preprocessor level, and the namespace definition in that file is empty and useless). Here's how the template looks:

namespace alx {

    template<std::size_t N, typename... Args>
    int sbprintf(char(&buff)[N], std::ptrdiff_t *written,
                 const char *fmt, Args... args)
    {
        return swnprintf(buff, written, sizeof buff, fmt, args...);
    }

}

Not only is this much neater than a macro, it's standard C++ (no GNU extensions), and automatically type-safe with no need for static asserts.

We can enable extra type checking on GCC using the format attribute, but we'd need to use <cstdarg> variable argument list rather than a template parameter pack for that.

I don't see the advantage of giving the functions different names; this is a situation where operator overloading allows us to use one name regardless of arguments (and allows natural extension to write wide character strings, for instance). We might choose to provide overloads for providing the written argument or not (by reference), as I can't see any need for this to be a run-time thing.

When std::vsnprintf() returns a negative result, we shouldn't be storing that in written - it makes more sense to store zero instead, and ensure we write an empty string.

I think the use of goto for error handling is less clear than simply putting the error return inline - each label has only one entry point, and there's no common cleanup to do.

The cast of len to unsigned is pointless and possibly harmful, given that std::ptrdiff_t is a signed type.

---

Modified code

With some of the improvements suggested above, I get the following.

Header

#include <cstdarg>
#include <cstddef>

#ifdef __GNUC__
#define attribute(x) __attribute__(x)
#else
#define attribute(x)
#endif

namespace alx {

    // va_list version
    int sprintf(char *str, std::ptrdiff_t *written,
                  std::ptrdiff_t buf_size, const char *format, va_list ap)
        attribute((format (printf, 4, 0)));

    // general version
    int sprintf(char *str, std::ptrdiff_t *written,
                  std::ptrdiff_t buf_size, const char *format, ...)
        attribute((format (printf, 4, 5)));

    // deduce size from buffer argument
    template<std::size_t N>
    int sprintf(char(&buff)[N], std::ptrdiff_t *written,
                 const char *fmt, ...)
        attribute((format (printf, 3, 4)));
}

#undef attribute

// template definition
template<std::size_t N>
int alx::sprintf(char(&buff)[N], std::ptrdiff_t *written,
                  const char *fmt, ...)
{
    va_list ap;
    va_start(ap, fmt);
    int result = alx::sprintf(buff, written, N, fmt, ap);
    va_end(ap);
    return result;
}

Implementation

#include <cerrno>
#include <cstdarg>
#include <cstddef>
#include <cstdio>

int alx::sprintf(char *str, std::ptrdiff_t *written,
                 std::ptrdiff_t buf_size, const char *format, va_list ap)
{
    if (buf_size <= 0) {
        return - (errno = EOVERFLOW);
    }

    int len = std::vsnprintf(str, buf_size, format, ap);

    if (len < 0) {
        *str = '\0';
        if (written) {
            *written = 0;
        }
        return -errno;
    }

    if (len >= buf_size) {
        if (written) {
            *written = buf_size - 1;
        }
        return - (errno = EOVERFLOW);
    }

    if (written) {
        *written = len;
    }

    return  0;
}

int alx::sprintf(char *str, std::ptrdiff_t *written,
                 std::ptrdiff_t buf_size, const char *format, ...)
{
    va_list ap;
    va_start(ap, format);
    int result = alx::sprintf(str, written, buf_size, format, ap);
    va_end(ap);
    return result;
}

Look Ma, no macros!

Answer 2

Welcome to the world of modern C++!

The OP has stated that "I don't think you need to write code that a C programmer couldn't understand to say that a program is C++." That's partially right — high-level, modern C++ techniques are easy to understand. What is wrong is that C++ programs don't have to be understood by C programmers in particular — C++ and C are different languages. If you are programming in the common subset of C and C++ (plus a few C++ features), you are not programming in C++.

As such, this answer focuses exclusively on how to convert your code to C++ code. It seems that the OP is not very familiar with modern C++ programming, so some of the points may not make much sense to the OP. As they get more familiar with C++ programming, hopefully they will understand.

---

After writing swnprintf and sbprintf in C (Encapsulating snprintf to avoid repetition of sizeof), I've written a C++ version of them:

Oh that's nice. Don't have to deal with low-level programming details anymore. Finally we have a C++ wrapper on top of snprintf.

swnprintf:

Similar to snprintf. Properties: (properties not mentioned shall be the same as snprintf)

• It writes the actual value of characters written (excluding the NUL terminator) through a pointer, instead of returning it.

Huh? I was expecting a C++-style function. Why are we using output parameters?

The user can explicitly discard that value by passing NULL.

Why not just return it then? Let's read on.

If a NUL character has been written in the middle of the string (eg.: swnprintf(buf, &w, sizeof(buf), "%c%c%c", 'a', 0, 'b');), the last NUL written is not counted, but any other characters until that one, including NULs are counted. In this example, w == 3 after the function call.

This makes sense. The NUL character is not included in the length of a string anyway.

If the string is truncated, the value written is the actual number of characters written, and not the value that would have been written had n been sufficiently large.

The phrase "the string is truncated" caught my eyes. Why do we have to care about that in C++?

If there's an error in the internal call to snprintf, the resulting string is unreliable (written is also unreliable) and the return value is negative. If there's truncation, the string is valid and the return value is positive. If there's no error, the return value is 0.

The reason for using an output parameter is revealed — you are using the return value to report errors. In C++, the usual way is to use the return value for output, and exceptions to report errors.

errno is set on any error. Truncation: ENOMEM. The absolute value of the return value is the same as errno.

Good old errno ... C++ programs are expected to stay a few meters away from it.

Usage:

• Example where the user doesn't care about truncation:

  char        buf[BUFSIZ];
  ptrdiff_t   tmp;
  ptrdiff_t   len;
  
  if (alx::swnprintf(buf, &tmp, sizeof(buf), "text, num=%i", 7) < 0)
          goto err;
  len = tmp;
  if (alx::swnprintf(&buf[len], &tmp, sizeof(buf) - len, "2nd part") < 0)
          goto err;
  len += tmp;
  

Wait, this is C — well, not really, we have ::. But this isn't the kind of C++ I am expecting — in C++ I expect this:

std::string result = alx::swnprintf("text, num=%i", 7) + alx::swnprintf("2nd part");

Notice:

• the conciseness and readability; (I doubt a C programmer has trouble understanding it)

• the reduced opportunity for errors to kick in;

• the lack of explicit memory management (std::string does them under the hood) and error checking (exceptions do them under the hood).

• Example where the user cares about truncation and doesn't about len:

  char        cmd[_POSIX_ARG_MAX];
  ptrdiff_t   tmp;
  
  if (alx::swnprintf(cmd, &tmp, sizeof(cmd), "%s ", "cat"))
          goto err;
  if (alx::swnprintf(&cmd[tmp], NULL, sizeof(cmd) - tmp, " %s ", "main.c"))
          goto err;
  system(cmd);
  

Corresponding example in a C++ program:

system(alx::swnprintf("%s %s", "cat", "main.c"));

Where system has been wrapped to take std::string:

int system(const std::string& command)
{
  return std::system(command.c_str());
}

sbprintf:

This is a higher abstraction than swnprintf. It is designed to only accept arrays as input. It is safer because it calculates internally the size of the buffer, so the user has less chance of writing buggy code. The down side is that it is less flexible (it can only write at the beginning of a buffer). Apart from that, the behaviour is the same as in swnprintf.

I would expect a "higher abstraction" to behave as in the above examples.

---

It is always easier said than done. Now let's convert your code step by step.

I am very glad to see that you used a namespace. This avoids name clashes.

Here's your swnprintf declaration:

int swnprintf(char *__restrict__ str, ptrdiff_t *__restrict__ written,
              ptrdiff_t nmemb, const char *__restrict__ format, ...);

I would separate it to two functions. One receives a length argument:

template <typename... Args>
std::string swnprintf(const char* format, std::size_t length, Args&&... args);

The other does not, and calls snprintf twice to determine the length: (I have no idea how to name it)

template <typename... Args>
std::string swprintf(const char* format, Args&&... args);

Notice that the output is stored in the returned std::string. length is of type std::size_t to prevent negative length in the first place. The str is allocated internally by std::string, written is available via str.size(), nmemb is no longer needed, and the return value is converted to exceptions. And we use a template parameter pack to pass the arguments to snprintf.

You need to define templates in the header. Here's my implementation:

namespace alx {

  struct Internal_error :std::exception {};
  struct Truncation_error :std::exception {};

  template <typename... Args>
  std::string swnprintf(const char* format, std::size_t length,
                        Args&&... args)
  {
    std::string buffer(length, '\0');
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wformat"
    int len = std::snprintf(buffer.data(), length + 1, format,
                            std::forward<Args>(args)...);
#pragma GCC diagnostic pop
    if (len < 0)
      throw Internal_error{};
    else if (static_cast<std::size_t>(len) <= length)
      return buffer;
    else
      throw Truncation_error{};
  }

  template <typename... Args>
  std::string swprintf(const char* format, Args&&... args)
  {
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wformat"
    int len = std::snprintf(nullptr, 0, format, args...);
#pragma GCC diagnostic pop
    return swnprintf(format, len, args...);
  }

}

Internal_error and Truncation_error are exception types to denote errors. (I had to add the pragmas to suppress the warning; in practice some kind of wrapping is expected.)

Answer 3

The error handling can be simplified to three return points instead of four: (removes one line)

        return  0;
trunc:
        if (written)
                *written = nmemb - 1;
        errno   = ENOMEM;
        return  ENOMEM;
neg:
        errno   = EOVERFLOW;
err:
        if (written)
                *written = 0;
        return  -errno;

Note: This includes Toby Speight's suggestion of clearing written

---

A better name for swnprintf would be snprintfs, given that it can replace every use case of snprintf with added safety.

---

The return value should always be used. Failure to do so shall be diagnosed:

__attribute__ ((warn_unused_result))