2

I'm trying to learn about the snprintf and found this answer with the example:

char buf[20] = "";
char *cur = buf, * const end = buf + sizeof buf;
cur += snprintf(cur, end-cur, "%s", "foo");
printf("%s\n", buf);
if (cur < end) {
    cur += snprintf(cur, end-cur, "%s", " bar");
}
printf("%s\n", buf);
free(str);

The thing that is not clear to me is that we allocate a fixed hardcoded buffer size which seems suffer from buffer overflow. In the N1570 I found that (7.21.6.5)

1

#include <stdio.h>
int snprintf(char * restrict s, size_t n,
const char * restrict format, ...);

2 The snprintf function is equivalent to fprintf, except that the output is written into an array (specified by argument s) rather than to a stream. If n is zero, nothing is written, and s may be a null pointer.

So to me it appears as the idiomatic usage would be as follows:

int need_space = snprintf(NULL, 0, "abs %s", "fgh") + 1; //How much to allocate?
char *const str = malloc(need_space * sizeof(char)); //allocate
int written = snprintf(str, need_space, "abs %s", "fgh"); //do format
printf("Need space = %d, written = %d\n", need_space, written);
printf("%s\n", str);

Or this is not common and has another problem?

7
  • It's got its second argument for a reason. This is an idiomatic C-way to try avoid overflows. Commented Jan 16, 2019 at 7:06
  • 3
    There is no "silent" truncation, see snprintf.3p - Linux manual page paying careful attention to the "RETURN" portion. You validate the return against the buffer size. Commented Jan 16, 2019 at 7:21
  • 3
    by definition sizeof(char) is 1 Commented Jan 16, 2019 at 7:38
  • 2
    One reason that what you show is not necessarily what people do is that there is a perceived cost to formatting the data twice, once to find out how long it will be and a second time to actually do the formatting. In your example with short fixed-length strings, the cost is not great. With more complex formats, more arguments, and especially longer string arguments, there is some cause for concern (but measurement would still be a good idea — how slow is it really?). Some systems (Linux, GNU C Library) have asprintf() and vasprintf(); using these can be a good idea. Commented Jan 16, 2019 at 7:52
  • 1
    Agree on previous comment and just small addition - your approach requires to specify the same formatting string twice, which in case of complex string is error-prone because declaring format strings with preprocessor defines is also a bad practice. Commented Jan 16, 2019 at 8:03

2 Answers 2

4

The 2x call to snprintf() is a common idiom.

... has another problem?

Problems lie in the corners

Format maintenance

The below suffers from a repeated format - prone to breaking as code ages and and only one line is changed.

// Oops!
int need_space = snprintf(NULL, 0, "abs %s", "fgh") + 1;
char *const str = malloc(need_space * sizeof(char));
int written = snprintf(str, need_space, "abs %s ", "fgh");

Did you notice the difference?

Better to state the format once.

#define FMT_ABS_S "abs %s"
int need_space = snprintf(NULL, 0, FMT_ABS_S, "fgh");
char *const str = malloc(sizeof *str * (need_space + 1u));
int written = snprintf(str, need_space + 1, FMT_ABS_S, "fgh");

Volatility

Code needs to insure the values do not change (non-volatile) between the 2 calls and special concern arise in multi-threaded applications.

Error checking

Code lacked checks. Yet even with checks, how to even handle unexpected results - perhaps just bail?

static const char *fmt_s = "abs %s";
int length = snprintf(NULL, 0, fmt_s, "fgh");
if (length < 0) {
  Handle_EncodingError();
}
char *str = malloc(sizeof *str * (length + 1u));
if (str == NULL) {
  Handle_OutOfMemory();
}
int written = snprintf(str, length + 1, fmt_s, "fgh");
if (written < 0 || written > length) {
  Handle_Error();
}
...
free(str);

Going twice though

2 calls can be wasteful in select situations, yet commonly the impact is less than thought. @Jonathan Leffler

Yet for me, the "what to do if allocation fails" is a show stopper anyways and code reports the error and maybe exit.

Selectively, once is enough

When the s*printf() format is tightly controlled, I see 1 call as sufficient.

// int to string

#define LOG2_N 28
#define LOG2_D 93
// Number of char needed for a string of INT_MIN is log10(bit width) + 3
#define INT_SIZE ((sizeof(int)*CHAR_BIT-1)*LOG2_N/LOG2_D + 3)

char s[INT_SIZE * 2];  // I like to use 2x to handle locale issues, no need to be stingy here.

int len = snprintf(s, sizeof s, "%d", i);

if (len < 0 || (unsigned) len >= sizeof s) {
  Handle_VeryUnusualFailure();
}
Sign up to request clarification or add additional context in comments.

2 Comments

I think here: if (written < 0 || written > needed_space) { one needs >= instead of >?
Confident '>' is correct, yet maybe length instead of space as the size needed is length + 1 to form the string.
0

In addition to @chux precise answer, it should be noted that the posted code has potential undefined behavior:

char buf[20] = "";                                // initialization is optional
char *cur = buf, * const end = buf + sizeof buf;  // cumbersome combined definitions
cur += snprintf(cur, end-cur, "%s", "foo");       // cur may be increased beyond the end of the array
printf("%s\n", buf);                              // no problem here
if (cur < end) {                                  // undefined behavior if cur points beyond the end of the array
    cur += snprintf(cur, end-cur, "%s", " bar");  // again cur may be increased too far.
}
printf("%s\n", buf);                              // OK
free(str);                                        // definition for str is not posted

If snprintf truncates the output, cur will be increased to point beyond the end of the array buf, which has undefined behavior. Comparing cur and end is meaningless if they do not point the same array or to the element just after the array.

It is easy to fix these problems:

char buf[20];
size_t size = sizeof buf;
size_t pos = 0;
int n = snprintf(buf + pos, size - pos, "%s", "foo");
if (n < 0) {
    // handle this unlikely error
    *buf = '\0';
} else {
    pos += n;
}
printf("%s\n", buf);
if (pos < size) {
    n = snprintf(buf + pos, size - pos, "%s", " bar");
}
printf("%s\n", buf);

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.