Avoiding repetition when using the "snprintf idiom" to write text

Question

I'm writing a function to stringify a nested struct. I'm using snprintf to pre-calculate how much space I'll need to allocate.

The problem is, it's extremely repetitious, and will only get worse if I ever need to add more fields to any of the structs. The duplication also creates more of an opportunity for accidental asymmetrical changes.

Is there an idiomatic way of reducing the duplication?

The repetitious code:

char* stringify_state(State* state) {
    Pet* p = state->pet;
    Settings* s = state->settings;

    char* format_string = "%lld %f %f %f %f %f %f %f %f %f %f";

    size_t len = snprintf(NULL, 0, format_string,
                          state->last_update_time,
                          p->health, p->max_health, p->satiation, p->max_satiation,
                          s->milliseconds_per_tick, s->hunger_pain_per_tick, s->hunger_per_tick, s->satiated_heal_per_tick,
                          s->pain_per_wrong_answer, s->satiation_per_right_answer);

    char* buff = malloc(len + 1);

    if (buff) {
        snprintf(buff, len + 1, format_string,
                 state->last_update_time,
                 p->health, p->max_health, p->satiation, p->max_satiation,
                 s->milliseconds_per_tick, s->hunger_pain_per_tick, s->hunger_per_tick, s->satiated_heal_per_tick,
                 s->pain_per_wrong_answer, s->satiation_per_right_answer);

        return buff;

    } else {
        return NULL;
    }
}

Context (Not what I'm asking about, but feel free to mention anything):

#include <time.h>
#include <stdlib.h>
#include <stdio.h>

typedef struct {
    double health;
    double satiation;

    double max_health;
    double max_satiation;

} Pet;

void init_pet(Pet* pet, double max_health, double max_satiation) {
    pet->max_health = max_health;
    pet->max_satiation = max_satiation;

    pet->health = max_health;
    pet->satiation = max_satiation;



typedef struct {
    double milliseconds_per_tick;

    double hunger_pain_per_tick;
    double hunger_per_tick;

    double satiated_heal_per_tick;

    double pain_per_wrong_answer;
    double satiation_per_right_answer;

} Settings;

void init_settings(Settings* s,
                   double ms_per_tick,
                   double hung_pain_per_tick, double hung_per_tick,
                   double sat_heal_per_tick,
                   double pain_per_wrong, double sat_per_right) {

    s->milliseconds_per_tick = ms_per_tick;

    s->hunger_pain_per_tick = hung_pain_per_tick;
    s->hunger_per_tick = hung_per_tick;

    s->satiated_heal_per_tick = sat_heal_per_tick;

    s->pain_per_wrong_answer = pain_per_wrong;
    s->satiation_per_right_answer = sat_per_right;
}



typedef struct {
    time_t last_update_time;
    Pet* pet;
    Settings* settings;

} State;

void init_state(State* state, Pet* pet, Settings* settings, time_t t) {
    state->pet = pet;
    state->settings = settings;

    state->last_update_time = t;
}

A test main:

int main() {
    Pet pet;
    init_pet(&pet, 100, 100);

    Settings settings;
    init_settings(&settings, 91, 82, 73, 64, 55, 46);

    State state;
    init_state(&state, &pet, &settings, 0);

    char* str = stringify_state(&state);

    if (str) {
        printf("%s", str);

        free(str);
    }
}

Prints

0 100.000000 100.000000 100.000000 100.000000 91.000000 82.000000 73.000000 64.000000 55.000000 46.000000

Answer 1

@vnp's code is solid and helpful, but his stringify_state_helper is a single-purpose function, and still leaves a degree of repetition and memory management in stringify_state. I'd rather have general-purpose to_string that takes printf-style arguments, allocates sufficient space for the converted result, and prints into that space, and returns the result:

#include <stdio.h>
#include <stdarg.h>
#include <stdlib.h>

char *to_string(char const *fmt, ...) { 
    va_list args;
    va_start(args, fmt);

    va_list args_dupe;
    va_copy(args_dupe, args);

    int size = vsnprintf(NULL, 0, fmt, args);

    char *ret = malloc(size+1);

    if (ret != NULL)
        vsnprintf(ret, size+1, fmt, args_dupe);

    va_end(args);
    va_end(args_dupe);

    return ret;
}

In fairness, this does require a little more code, and the v*printf functions (and argument-list macros) are a bit less known, so some may find it a bit more difficult to understand.

On the other hand, in exchange for that bit of extra investment, we get something that's more general, and works much more as I think most people would expect--for example, something like this:

char *s = to_string("%d, %d", 1, 2);

Using this, stringify_state should turn out something like this:

char* stringify_state(State* state) {
    Pet* p = state->pet;
    Settings* s = state->settings;

    return to_string("%lld %f %f %f %f %f %f %f %f %f %f",
                     state->last_update_time,
                     p->health, 
                     p->max_health, 
                     p->satiation, 
                     p->max_satiation,
                     s->milliseconds_per_tick, 
                     s->hunger_pain_per_tick, 
                     s->hunger_per_tick,
                     s->satiated_heal_per_tick,
                     s->pain_per_wrong_answer,
                     s->satiation_per_right_answer);
}

Levels of abstraction

It may just be a pet peeve of mine, but I think this divides into cleaner layers of abstraction. With your original code, we have a single function that mixes a fairly high level of abstraction (format a State into a string) with lower levels of abstraction (e.g., memory management).

@vnp's code sort of inverts the layers of abstraction, so we have the higher level of abstraction at the bottom, and the lower level of abstraction above it.

This one gets the layers of abstraction closer to how I'd like to see them: the memory management and general purpose stringifying is at the bottom of the stack, and the more specific higher level concern of Stringifying a State is at an upper layer. So, something like a call graph accurately reflects the levels of abstraction being dealt with in the code.

Other Points

1. Given the number (and length) of arguments you're passing, I'd prefer to see the arguments passed one per line as I've formatted them above, rather than a variable number per line to justify out to some particular right margin.

2. Since stringify_state isn't intended to modify the State whose address is being passed, it's probably better to define the parameter as a State const *. Likewise with the local variables, so the first lines would probably be better written something like this:

   char* stringify_state(State const* state) {
       Pet const* p = state->pet;
       Settings const* s = state->settings;        
   

Answer 2

Consider factoring it out into a function:

static size_t stringify_state_helper(State * state, char * buf) {
    return snprintf(buf, ....);
}

char * stringify_state(State * state) {
    size_t len = stringify_state_helper(state, NULL);
    char * buf = malloc(len + 1);
    if (buf) {
        stringify_state_helper(state, buf);
    }
    return buf;
}

Answer 3

The GNU libc already provides asprintf, which avoids exactly the repetition you are concerned about. I haven't looked at the solutions of other operating systems (Windows, BSD, AIX, Solaris, embedded), though I hope they provide something equivalent.

Answer 4

Write the ifs such that the error paths are inside them, and the error-free path isn't indented:

buff = malloc(len + 1);
if (buff) {
        snprintf(buff, len + 1, format_string,
                        state->last_update_time,
                        p->health, p->max_health,
                        p->satiation, p->max_satiation,
                        s->milliseconds_per_tick,
                        s->hunger_pain_per_tick,
                        s->hunger_per_tick,
                        s->satiated_heal_per_tick,
                        s->pain_per_wrong_answer,
                        s->satiation_per_right_answer);
        return buff;
} else {
        return NULL;
}

Is cleaner this way:

buff = malloc(len + 1);
if (!buff)
        return NULL;
written = snprintf(buff, len + 1, format_string,
                        state->last_update_time,
                        p->health, p->max_health,
                        p->satiation, p->max_satiation,
                        s->milliseconds_per_tick,
                        s->hunger_pain_per_tick,
                        s->hunger_per_tick,
                        s->satiated_heal_per_tick,
                        s->pain_per_wrong_answer,
                        s->satiation_per_right_answer);
if (written < 0)
        goto err;
return buff;

err:
free(buff);
return NULL;

---

Edited (02/jul/19 after Will comment) to add error checking

Note that snprintf may fail (EILSEQ or EOVERFLOW)

Answer 5

There's nothing wrong with the other answers, but if you'd prefer a solution more portable than asprinf() and slightly more straightforward than Jerry Coffin's vsnprintf() helper function solution, consider this variable length macro solution:

#define FOO_ASPRINTF(_str, ...) /* replace FOO with your codebase's prefix */ \
do { \
    int byte_c = snprintf(NULL, 0, __VA_ARGS__); \
    (_str) = malloc(byte_c + 1); \
    if (!(_str)) { \
        FOO_MALLOC_ERR; /* should depend on your codebase's malloc() policy */ \
    } \
    sprintf(_str, __VA_ARGS__); \
} while (0)

Which in OP's case might be invoked as:

char * buff = NULL;
CG_ASPRINTF(buff,
    "%lld %f %f %f %f %f %f %f %f %f %f",
    state->last_update_time,
    p->health,
    p->max_health,
    p->satiation,
    p->max_satiation,
    s->milliseconds_per_tick,
    s->hunger_pain_per_tick,
    s->hunger_per_tick,
    s->satiated_heal_per_tick,
    s->pain_per_wrong_answer,
    s->satiation_per_right_answer
);

Some people might be allergic to macros, but in this case it allows easy implementation, avoids any overhead, and is perfectly portable.

Answer 6

Side issue

"%f" does not well express the state of a floating point (FP) object

When a FP is large, printing 100s of digits is not informative. More compact options exist.

Worse, when a FP is much smaller than 1, "%f" only retains a few or zero digits - losing perhaps all precession.

As FP are encoded in exponential notation, use an exponential format

printf("%a", fp);  //Hexadecimal significant with binary exponent
// or 
printf("%.*e", DBL_DECIMAL_DIG - 1, fp);  // Decimal exponential
// or 
printf("%.*g", DBL_DECIMAL_DIG, fp);  // Decimal exponential when needed, else fix point.

ref

---

Minor

snprintf() returns an int.

Capture result with an int which may be negative due to "negative value if an encoding error occurred."

int ilen = snprintf(....
if (ilen < 0) return NULL; // or some other error indication.

size_t size = ilen + 1u;
char* buff = malloc(size);
...