vsnprintf with a null destination pointer unexpected answer

Question

Main Issue

I'm getting some unexpected result when using vsnprintf. In the code below, I used snprintf and passed a null destination pointer to find out how much space it needs

#define long_string 256
typedef char STRING_VARIABLE [long_string + 1];

void SAFE_snprintf( char * buffer, char * format ,...  )
{
  va_list args;

  va_start(args, format);

  int m = vsnprintf (0, 0, format, args);
  printf("m = %d\n", m);
  if (m < 0)
    {
    perror ("snprintf failed");
    abort ();
    }

  // Allocating memory
  char *bufferString = (char *) malloc (n - 1);
  if (!bufferString)
    {
    perror ("malloc failed");
    abort ();
    } 

  m = vsnprintf (bufferString, (n - 1), format, args);

 if (m < 0)
    {
    perror ("vsnprintf failed");
    abort ();
    }

  strcpy(buffer, bufferString);

  free (bufferString);

  va_end(args);

}

int main(int argc, char * argv[])
{
  char InputString [] = "Hello";
  STRING_VARIABLE bufferStrings;
  char format [] = "%s_test";
  int n = snprintf (0, 0, "%s_test", InputString);

  if (n < 0)
    {
    perror ("vsnprintf failed");
    abort ();
    }

  printf("n = %d", n);
  

  SAFE_snprintf(bufferStrings, format , InputString);
  
  return 0;
}

The above code returns

n = 7
m = 10

I'm not sure why snprintf is returning 7 (which is correct), and vsnprintf is returning 10. Which I assume is wrong or of course my understanding is flawed somewhere.

Why I am doing this?

I wanted to use snprintf "safely" i.e. by avoiding string truncation. The idea was to determine the string size before using snprintf. This meant using it twice. Once to workout the size, then allocate appropriate memory to use snprintf again. Ofcourse there are a variable amount of inputs needed as any printf functions. So wanted to use vsnprintf to create a variadic function which does the aforementioned.

I know there is still the issue of checking if the original string being passed isn't too long and will not result in string truncation. But confused to why vsnprintf is not working as expected

Answer 1

It's invalid to re-use va_list for another call.

va_list args;
va_start(args, format);
vsnprintf(..., args);
va_end(args); // basically you have to call it

Call va_copy or va_start after it again before calling another v* function.

va_list args;
va_start(args, format);
vsnprintf(..., args);
va_end(args);
va_start(args, format);
vsnprintf(..., args);
va_end(args);

or

va_list args, args2;
va_start(args, format);
vsnprintf(..., args);
va_copy(args2, args);
va_end(args);
vsnprintf(..., args2);
va_end(args2);

Your buffer is limited to 256 characters long_string 256, so it does not solve any problem and strcpy(buffer, bufferString); is very unsafe.

Do not use typedef arrays - they are very confusing. Prefer a structure.

Overall, you seem to want to implement asprintf - see sprintf() with automatic memory allocation? , https://man7.org/linux/man-pages/man3/asprintf.3.html . Maybe asprintf is going to be standard https://en.cppreference.com/w/c/experimental/dynamic .

Answer 2

There are multiple problems in your code:

• you reuse args without restarting the va_list with va_start() or va_copy().
• you allocate n - 1 bytes instead of n + 1.
• copying the allocated string with strcpy is unsafe as the composed string may exceed 256 bytes.
• format should have type const char *format.
• your type STRING_VARIABLE is very confusing. Hiding ponters behind typedefs is confusing, but hiding arrays behind typedefs is even worse.

Why not always allocate memory and return the pointer to the caller? Some C libraries have a function asprintf with these semantics, which is a good candidate for standardization, but it took 30 years for the Committee to consider strdup(), so making your own makes sense.

Here is an allocating version:

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

// allocating version of snprintf
char *SAFE_snprintf(const char *format, ...) {
    // use a local buffer to avoid calling snprintf twice for short strings
    char buf[256];
    va_list args;
    int len;

    va_start(args, format);
    len = vsnprintf(buf, sizeof buf, format, args);
    va_end(args);

    if (len < 0) {
        return NULL;    // errno was set by snprintf
    }

    // Allocating memory
    char *bufferString = (char *)malloc(len + 1);
    if (!bufferString) {
        return NULL;    // errno was set by malloc to EMEM
    }
    if (len < (int)sizeof(buf)) {
        strcpy(bufferString, buf);
    } else {
        va_start(args, format);
        vsnprintf(bufferString, len + 1, format, args);
        va_end(args);
    }
    return bufferString;
}

int main(int argc, char *argv[]) {
    char InputString[] = "Hello";

    int n = snprintf(NULL, 0, "%s_test", InputString);
    printf("n = %d\n", n);
  
    char *str = SAFE_snprintf("%s_test", InputString);
    if (str == NULL) {  
        perror("SAFE_snprintf failed");
        return 1;
    } else {
        printf("len=%zu, str=%s\n", strlen(str), str);
        free(str);
        return 0;
    }
}

If your goal is simply to detect truncation, just compare the return value of snprintf with the length of the destination array:

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

// safe version of snprintf that detects truncation
// complain on error and truncation
// return -1 on error
// return 1 on case of truncation
// otherwise return 0
int SAFE_snprintf(char *dest, size_t size, const char *format, ...) {
    int len;
    va_start(args, format);
    len = vsnprintf(dest, size, format, args);
    va_end(args);
    if (len < 0) {
        perror("vsnprintf failed");
        return -1;
    } else
    if (len < (int)size) {
        // no error
        return 0;
    } else {
        perror("vsnprintf caused truncation");
        return -1;
    }
}

int main(int argc, char *argv[]) {
    char hello[] = "Hello";
    char long_hello[] = "Pardon my intrusion, I merely want to say hello";
    char name[] = "Mrs Robinson";
    char buf[32];
    int res;

    res = SAFE_snprintf(buf, sizeof buf, "%s %s", hello, name);
    printf("res=%d, buf=%s\n", res, buf);
    res = SAFE_snprintf(buf, sizeof buf, "%s %s", long_hello, name);
    printf("res=%d, buf=%s\n", res, buf);
    return 0;
}