C variadic wrapper

Question

To output formatted debug output, I've written a wrapper for vsfprint. Now, I wanted to allocate exactly enough memory for the output buffer, instead of just claiming a random high buffer size (it's a small embedded platform (ESP8266)). For that I iterate through the variable arguments until a NULL is found.

This works fine, provided that I don't forget to add a (char *)NULL parameter to every call. So, I thought, let create another wrapper, a function that just relays all arguments and adds a (char *) NULL parameter:

#include <stdarg.h>
#include <stdio.h>
#include <string.h>
#include <stdlib.h> // malloc

void write_log(const char *format, ...) {

  char* buffdyn;
  va_list args;

  //  CALC. MEMORY
  size_t len;
  char *p;

  if(format == NULL)
    return;

  len = strlen(format);

  va_start(args, format);

  while((p = va_arg(args, char *)) != NULL)
    len += strlen(p);

  va_end(args);
  // END CALC. MEMORY

  // ALLOCATE MEMORY
  buffdyn = malloc(len + 1);    /* +1 for trailing \0 */
  if(buffdyn == NULL) {
    printf("Not enough memory to process message.");
    return;
  }

  va_start(args, format);
  //vsnprintf = Write formatted data from variable argument list to sized buffer
  vsnprintf(buffdyn, len, format, args);
  va_end(args);

  printf("%s\r\n",buffdyn);
  free(buffdyn);
}

void write_log_wrapper(const char *format, ...) {

  va_list arg;

  va_start(arg, format);
  write_log(format,arg,(char *)NULL);
  va_end(arg);
}


int main()
{
    const char* sDeviceName = "TEST123";
    const char* sFiller1 = "12345678";

    write_log_wrapper("Welcome to %s%s", sDeviceName,sFiller1);
    write_log("Welcome to %s%s", sDeviceName,sFiller1, (char *)NULL);

    return 0;
}

Calling the write_log() function directly works fine (if you don't forget the NULL parameter). Calling the write_log_wrapper() function will only display the first paramter, and then adds a "(nu" (garbage?) to the output.

What am I doing wrong? Is this a good way to approach what I'm aiming to do in the first place?

Thanks.

If you intend to write it to `stdout`, why don't you simply use `vprintf`? The same follows if you want to write to a file (use `vfprintf`). Then there is no buffer allocation in your code at all and you don't have to deal with figuring out how many arguments or how big of a buffer. — Dark Falcon, Jun 21 '16 at 14:33
You're ignoring the possibility that one of the arguments passed isn't a NUL-terminated string passed via a `char *`. What if you get an `int`, or a `double`? — Andrew Henle, Jun 21 '16 at 14:42
Reason why is, and you can't see that from this simplified example, is that the function does more than simply spit out the buffer. It also adds a timestamp, and, via an extra parameter, adds some ANSI coloring depending on that parameter ;-). But I wanted to keep the example simple ;-) — svenema, Jun 21 '16 at 16:18

Andrew Henle · Accepted Answer · 2016-06-21T15:17:58.910

To determine how big a buffer is needed to hold the output string, you need to fully parse the entire format string and actually expand the arguments.

You can either do it yourself, duplicating all the processing of printf() and its ilk and hoping to not make any mistakes, or you can use vsnprintf() - first to determine the size, and then to actually expand the inputs to one output string.

#define FIXED_SIZE 64

void write_log(const char *format, ...)
{
    // set up a fixed-size buffer and a pointer to it
    char fixedSizeBuffer[ FIXED_SIZE ];
    char *outputBuffer = fixedSizeBuffer;

    // no dynamic buffer yet
    char *dynamicBuffer = NULL;

    // get the variable args
    va_list args1;
    va_start( args1, format );

    // need to copy the args even though we won't know if we
    // need them until after we use the first set
    va_list args2;
    va_copy( args2, args1 );

    // have to call vsnprintf at least once - might as well use a small
    // fixed-size buffer just in case the final string fits in it
    int len = vsnprintf( fixedSizeBuffer, sizeof( fixedSizeBuffer ), format, args1 );
    va_end( args1 );

    // it didn't fit - get a dynamic buffer, expand the string, and
    // point the outputBuffer pointer at the dynamic buffer so later
    // processing uses the right string
    if ( len > sizeof( fixedSizeBuffer  ) )
    {
        dynamicBuffer = malloc( len + 1 );
        vsnprintf( dynamicBuffer, len + 1, format, args2 );
        outputBuffer = dynamicBuffer;
    }

    va_end( args2 );

    // do something with outputBuffer

    free( dynamicBuffer );
    return;
}

good idea but there are systems where you must not use `args` twice (e.g. linux on IBM Z series) so you should add a `va_copy()` — Ingo Leonhardt, Jun 21 '16 at 15:12
A very solid solution, tested it and it works out of the box :D. — svenema, Jun 21 '16 at 16:16

alk · Answer 2 · 2016-06-21T14:57:21.180

What am I doing wrong?

Passing a va_list arg

write_log(format, arg, (char *)NULL);

is not the same as passing several char*

write_log("Welcome to %s%s", sDeviceName, sFiller1, (char *)NULL);

You won't get around to pass a sentinel marking the end of the parameters passed, that is a (char*) NULL or whatever you decide to use.

Alternatives would be to

pass the number of arguments explicitly, perhaps as 2nd parameter
parse the format string for conversion specifiers, in fact mimicking what printf does.

score 1 · Answer 3 · answered Jun 21 '16 at 15:07

1

If you want just to ensure that all calls receive a setinel at the end, use a macro:

#define WRITE_LOG(...) write_log(__VA_ARGS__, (char*)0)

This ensures that there is always an extra 0 at the end.

Also be careful with NULL. It is underspecified in the C standard to what expressions this resolves. Common cases are 0 and (void*)0. So on 64bit architectures these may have different width (32 bit for the first, 64 bit for the second). It can be deadly for a variadic function to receive the wrong width here. Therefore I used (char*)0 which is the type that your function seems to expect. (But (void*)0 would also do in this special case.)

answered Jun 21 '16 at 15:07

Jens Gustedt

76,821
6
102
177

Given that the target platform is "a small embedded platform", I'd recommend the OP verify that `(char*)0` is in fact a `NULL` pointer for the compiler being used before using at as such. I don't think `(char*)0` was mandated to be a valid `NULL` pointer until C11. – Andrew Henle Jun 21 '16 at 15:13
@AndrewHenle, you probably mean null pointer and not `NULL` pointer. `(char*)0` is not a "null pointer constant" in the sense of the standard, here you would in effect need `(void*)0`. But for the case at hand this is irrelevant, it is a null pointer of the right type as the program expects. – Jens Gustedt Jun 21 '16 at 15:55
It sometimes works, sometimes crashes.. looks like to depend on the number of arguments.. Can't really understand why yet. – svenema Jun 21 '16 at 16:30

C variadic wrapper

3 Answers3