stdarg.h functionality appears to concatenate arguments wrongly

Question

TL;DR: mprintf("%s and %s", arg1, arg2) appears to print "arg1arg2 and arg2" instead of "arg1 and arg2" using va_* macros defined in stdarg.h

Hello, all. I'm using the functionality provided by vsprintf like so:

exits.c

#include "../../utils/printutil.c"

...

char dir[4][5];
char* format;

...

switch(bitcount) {

    ...

    case 2: format = "%s and %s";
        mprintf(format, dir[0], dir[1]);
    break;

    ...
}

Note: dir gets its values from strcpy(dir[bitcount], names[dircount]);, where names is simply a char pointer array such that dir[0] = "North", dir[1] = "East", dir[2] = South", and dir[3] = "West".

---

printutil.c

/* Utils file to provide common utilities not tied specifically to any aspect of the software */

#include "printutil.h"

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

char* mprintf(const char * format, ...) {
/* Prints a statement containing a format and then multiple arguments. */

    char *buf;
    va_list args;

    va_start (args, format);
    buf = malloc(sizeof(format) + sizeof(args));
    vsprintf (buf, format, args);
    va_end (args);

    return buf;
}

Note: that printutil.h simply contains function prototypes

---

So that's the structure of the code. During a case of the switch statement, a format string is specified, and then mprintf() (defined in printutil.c) is called with the format and the args dir[0] and dir[1], two variables that were successfully written to earlier in exits.c.

Using gdb, I was able to discern that the values being passed to mprintf() were as expected:

Breakpoint 1, exitstr (exits=5 '\005') at exits.c:33
33          switch(bitcount) {
(gdb) s
38              case 2: format = "%s and %s";
(gdb) s
39                  mprintf(format, dir[0], dir[1]);
(gdb) p format
$1 = 0x403115 "%s and %s"
(gdb) p dir[0]
$2 = "North"
(gdb) p dir[1]
$3 = "South"

When I step into the mprintf() function, gdb shows the contents of format are exactly as they should be, and shows the content of the va_list args as so:

17          vsprintf (buf, format, args);
(gdb) p format
$4 = 0x403115 "%s and %s"
(gdb) p args
$5 = {{gp_offset = 8, fp_offset = 48, overflow_arg_area = 0x7fffffffe710,
    reg_save_area = 0x7fffffffe650}}

I built this code based on examples found in the cplusplus.com reference for vprintf and vsprintf and they both indicate that I have used the functions and the macros defined in stdarg.h correctly.

However, after stepping over the vsprintf() line, printing the contents of buf yields the source of my problems. Namely that the second argument is seemingly concatenated to the first, and then the second argument is re-used for the second format specifier.

(gdb) print buf
$7 = 0x63ca50 "NorthSouth and South"

Strangely, this only appears to happen when either 'North' or 'South' are the first arguments. If 'East' or 'West' are the first arguments, the arguments are printed to buf correctly.

---

Thank you all, in advance, for your time and patience.

Answer 1

buf = malloc(sizeof(format) + sizeof(args));

What is this supposed to do? sizeof (format) is just the size of a pointer, 4 bytes for 32-bit system or 8 bytes for 64-bit. sizeof (args) is just another name for sizeof (va_list), which is an implementation-defined type. However, you are using this as the expected size of a string.

Likely you will overflow this buffer and run into undefined behavior.

It's always better to use the variants of snprintf, which take a specified output buffer size.

Edit: Additionally, as @Mahonri noticed, you have put the string "North" into an array with space for only 5 characters and this discarded the terminating NUL byte. This caused sprintf to overrun the intended end of the string. I would have expected it to print NorthEast instead, but it's still only undefined behavior.

Answer 2

On recent enough systems, snprintf() tells you the number of bytes required even if it doesn't anything else. That makes your code as easy as

char* mprintfv(const char * format, va_list ap) {
    // This version should always be given for the case someone wants to build on it.

    va_list ap2;

    va_copy(ap2, ap);
    size_t length = vsprintf(NULL, format, ap2);
    va_end(ap2);
    if (size_t < 0) return NULL;
    char *buf = malloc(length + 1);
    if (!buf) return NULL;
    vsprintf(buf, format, ap2);
    return buf;
}

char* mprintf(const char * format, ...) {
    /* Prints a statement containing a format and then multiple arguments. */

    va_list args;

    va_start (args, format);
    char *buf = mprintfv(format, args);
    va_end(args);

    return buf;
}

On older systems, however, it might happen that sprintf() returns without telling how much space is needed. In this case, you'd have to create a loop which successively grows your memory block until it works, and resize it again.