Null-terminating a list of arguments for a C variadic function

Question

I'm messing around with variadic functions in C to learn how they work, and am trying to build a simple 'print lines' function without requiring manual counting of the lines. I'm doing this by wrapping the function in a macro that adds a null pointer to the end of a list of char * arguments, so the function can print line-by-line until a null argument is found.

I know I've avoided some common pitfalls, like forgetting to cast the null pointer in the argument list, but for whatever reason this code still isn't working. Calling the function with any number of parameters prints them properly, then fails to detect the null, prints a bunch of garbage data, and crashes.

int printline(const char *str) {
    printf("%s\n", str);
}

#define printlines(...) _comments(__VA_ARGS__, (char*)0)
int _printlines(char* first, ...) {
    if (first) {
        printline(first);

        va_list ptr;
        va_start(ptr, first);

        char *next;

        do {
            char *next = va_arg(ptr, char *);
            if (next) {
                printline(next);
            }
        } while(next);

        va_end(ptr);
    }
}

int main() {
    printlines("hi");
    //prints 'hi', then prints garbage data and crashes

    printlines("how", "are", "you");
    //prints 'how', 'are', and 'you', then prints garbage data and crashes
    
    _printlines("help", (char *)0);
    //prints 'help', then prints garbage data and crashes

    _printlines("something", "is", "wrong", (char *)NULL);
    //prints 'something', 'is', and 'wrong', then prints garbage data and crashes
}

Answer 1

If you take a look at this:

    char* next;
    do{
        char* next = va_arg(ptr,char*);
        if(next){ comment(next); }
    }while(next);

You'll see that you have two separate variables called next, with the one inside of the do..while loop masking the one defined outside. You're assigning the result of va_arg to the inner next. Then when you get the while (next) condition, the inner next is out of scope and you're now reading the outer next which was never written to. This triggers undefined behavior.

You instead want:

    char* next;
    do{
        next = va_arg(ptr,char*);
        if(next){ comment(next); }
    }while(next);

So that you only have a single variable called next that you're using.

Answer 2

Small rewrite. The macro has been modified with a +0 so it can take zero arguments.

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

#define printlines(...) _printlines(__VA_ARGS__+0,(void*)0)
void _printlines(const char * first, ...)
{
    const char * ptr;
    va_list va;
    va_start (va, first);

    printf("---begin---\n");
    for (ptr = first; ptr != NULL ; ptr = va_arg(va,char*) )
    {
        printf("%s\n", ptr);
    }
    printf("---end---\n");
    va_end(va);
}

int main()
{
    printlines();     // instead of: printlines(NULL);
    printlines("hi");
    printlines("how","are","you");
    return 0;
}

Answer 3

Save time, enable all complier warnings.

• warning: 'next' may be used uninitialized [-Wmaybe-uninitialized] } while(next); quickly gets to the key issue.

• warning: control reaches end of non-void function [-Wreturn-type] in 2 places.

This is faster than posting on stack overflow.

Answer 4

The "rubbish" comes from not initialized object next. Another one next defined in the loop stops to exist when you exit the loop.

Removing strange functions and cleaning some mess you can get right.

int printline(const char* str){
    printf("%s",str);
}

#define printlines(...) printlinesfunc(__VA_ARGS__,(char*)0)
int printlinesfunc(const char* first, ...){
    if(first)
    {
        va_list ptr;
        va_start(ptr,first);

        char* next;
        printline(first);
        while((next = va_arg(ptr, char *)))
            printline(next);
        va_end(ptr);
    }
}

int main(){
    printlines("hi" , "\n");
    printlines("how"," are"," you", "\n");
    printlines("help", "\n");
    printlines("something", " is", " wrong", "\n");
}

Answer 5

I highly recommend that you avoid variadic functions and use pointer arrays and variadic macros instead (with a terminator object).

Your function would have looked like this when using this approach:

void printline(const char *str) { printf("%s\n", str); }

int printlines(char **lines) {
  if (!lines)
    return -1;
  while (*lines)
    printline(*(lines++));
  return 0;
}

#define printlines(...) printlines((char *[]){__VA_ARGS__, NULL})

Not only are variadic functions sometimes difficult to code, but the ABI for variadic functions is problematic to the point that different languages might treat it differently and C bindings between different languages might break your code.

Besides, when using this approach, things can become much more fun and interesting as well, allowing for easy type detection and multi-type arguments... this code from the facil.io CSTL library provides a good example for what I mean.

The function accepts an array of structs:

/** An information type for reporting the string's state. */
typedef struct fio_str_info_s {
  /** The string's length, if any. */
  size_t len;
  /** The string's buffer (pointer to first byte) or NULL on error. */
  char *buf;
  /** The buffer's capacity. Zero (0) indicates the buffer is read-only. */
  size_t capa;
} fio_str_info_s;

/** memory reallocation callback. */
typedef int (*fio_string_realloc_fn)(fio_str_info_s *dest, size_t len);

/** !!!Argument type used by fio_string_write2!!! */
typedef struct {
  size_t klass; /* type detection */
  union {.      /* supported types */
    struct {
      size_t len;
      const char *buf;
    } str;
    double f;
    int64_t i;
    uint64_t u;
  } info;
} fio_string_write_s;

int fio_string_write2(fio_str_info_s *restrict dest,
                      fio_string_realloc_fn reallocate, /* nullable */
                      const fio_string_write_s srcs[]);

Then a macro makes sure the array's last element is a terminator element:

/* Helper macro for fio_string_write2 */
#define fio_string_write2(dest, reallocate, ...)            \
  fio_string_write2((dest),                                 \
                    (reallocate),                           \
                    (fio_string_write_s[]){__VA_ARGS__, {0}})

Additional helper macros were provided to make the fio_string_write_s structs easier to construct. i.e.:

/** A macro to add a String with known length to `fio_string_write2`. */
#define FIO_STRING_WRITE_STR2(str_, len_)                    \
  ((fio_string_write_s){.klass = 1, .info.str = {.len = (len_), .buf = (str_)}})

/** A macro to add a signed number to `fio_string_write2`. */
#define FIO_STRING_WRITE_NUM(num)                            \
  ((fio_string_write_s){.klass = 2, .info.i = (int64_t)(num)})

And the function used the terminator element to detect the number of arguments received by the macro:

int fio_string_write2 (fio_str_info_s *restrict dest,
                               fio_string_realloc_fn reallocate, /* nullable */
                               const fio_string_write_s srcs[]) {
  int r = 0;
  const fio_string_write_s *pos = srcs;
  size_t len = 0;

  while (pos->klass) {
    switch (pos->klass) { /* ... */ }
    /* ... counts total length */
    ++pos;
  }
  /* ... allocates memory, if required and possible ... */
  pos = srcs;
  while (pos->klass) {
    switch (pos->klass) { /* ... */ }
    /* ... prints data to string ... */
    ++pos;
  }
    /* ... house-keeping + return error value ... */
}

Example use (from the source code comments):

 fio_str_info_s str = {0};
 fio_string_write2(&str, my_reallocate,
                     FIO_STRING_WRITE_STR1("The answer is: "),
                     FIO_STRING_WRITE_NUM(42),
                     FIO_STRING_WRITE_STR2("(0x", 3),
                     FIO_STRING_WRITE_HEX(42),
                     FIO_STRING_WRITE_STR2(")", 1));

This both simplifies the code and circumvents a lot of the issues with variadic functions. This also allows C bindings from other languages to work better and the struct array to be constructed in a way that is more idiomatic for the specific target.