4

I have a function; 

void foo(const char* format, ...)
{
    char buffer[1080];

    // Supposed way to handle C Variable Arguments?
    va_list argptr;
    va_start(argptr, format);
    sprintf(buffer, format, argptr);
    va_end(argptr);

    printf_s("%s.\n", buffer);
}

int main()
{
    int val = 53;
    foo("%d", val);
}

Everytime I run this, I get MASSIVE numbers that change every during each run. 12253360, 5306452, etc. I don't understand why.

Is it something with my sprintf call or is it the way I'm doing the va_list argptr;? Is my buffer too large?

Thanks.

Hatefiend
  • 3,416
  • 6
  • 33
  • 74
  • `printf_s()`? Why? – Andrew Henle Mar 30 '17 at 22:06
  • `sprintf` does not accept an argument of type `va_list`. – Keith Thompson Mar 30 '17 at 22:07
  • @Andrew Henle My compiler gives me a warning if I use `printf` claiming it is unsafe and that I should be using `printf_s` instead. – Hatefiend Mar 30 '17 at 22:09
  • @Hatefiend *My compiler gives me a warning if I use `printf` ...* That's garbage that results in non-standard, non-portable code. See http://stackoverflow.com/questions/14386/fopen-deprecated-warning and http://stackoverflow.com/questions/3317536/visual-studio-warning-c4996 It almost seems *designed* to get inexperienced developers to write non-portable code.... – Andrew Henle Mar 30 '17 at 22:38
  • @Hatefiend To make it clear, the function your compiler is warning you about as being "deprecated" and "unsafe" is required by [the C Standard](http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf) as part of the C language: http://port70.net/~nsz/c/c11/n1570.html#7.21.6.3 `printf()` is no more "deprecated" in standard C than the `=` assignment operator. – Andrew Henle Mar 30 '17 at 22:55
  • @AndrewHenle If what you say is true then why would Visual Studio have gone and deprecated many of the standard c functions? – Hatefiend Mar 30 '17 at 23:06
  • @Hatefiend Why promote non-standard, non-portable functions over portable ones required by the language standard? One possible answer: https://en.wikipedia.org/wiki/Vendor_lock-in Note well the first example... – Andrew Henle Mar 30 '17 at 23:15
  • @AndrewHenle I had no idea printf_s and similar functions were Visual Studio exclusive. How do I just download C compiler that doesn't have any extra crap in it? – Hatefiend Mar 30 '17 at 23:19
  • @Hatefiend: Functions with `..._s` syntax are part of C standard as optional library components. I.e. their interface and behavior are standardized, but they are not guaranteed to be provided by the library. So, they are not entirely "non-standard". But code that uses these functions will not be portable. – AnT stands with Russia Mar 31 '17 at 00:56
  • @AnT I think you're understating the impact of the warnings and errors given when using functions that the standard *requires*. The fact that an implementation provides optional `..._s` functions does not mean the required functions are unsafe and deprecated - as I stated earlier, those required functions are no more unsafe or deprecated than the assignment operator. IMO those warnings are deliberately misleading. – Andrew Henle Mar 31 '17 at 14:06
  • @Hatefiend *How do I just download C compiler that doesn't have any extra crap in it?* On Windows? Unfortunately there's no easy way there. Windows is different, and its development ecosystem is huge. IMO Microsoft provides some absolutely great development tools, but with a catch: they all help reinforce "Windows-only". I just had to learn to watch out for the Windows-only extensions and avoid them. Use other platforms. Download VirtualBox and install Linux, BSD, and/or Solaris VMs. Program some on those. Use GCC. Download and try the Solaris Studio compiler from Oracle (works on Linux too). – Andrew Henle Mar 31 '17 at 14:16

1 Answers1

9

The technique you are apparently attempting to use suggests that you need vsprintf (or, better, vsnprintf)

va_list argptr;
va_start(argptr, format);
vsnprintf(buffer, sizeof buffer, format, argptr);
va_end(argptr);

That's the very reason such functions from v... group exist in standard library.

Calling sprintf the way you do it makes no sense at all - it cannot be used with an externally supplied va_list.

If you want to implement a variable-sized buffer you can do it as follows

void foo(const char* format, ...)
{
    static char *buffer;
    static size_t buffer_size;

    va_list argptr;
    va_start(argptr, format);
    int length = vsnprintf(buffer, buffer_size, format, argptr);
    va_end(argptr);

    if (length + 1 > buffer_size)
    {
      buffer_size = length + 1;
      buffer = realloc(buffer, buffer_size);
      /* Yes, `realloc` should be done differently to properly handle
         possible failures. But that's beside the point in this context */

      va_start(argptr, format);
      vsnprintf(buffer, buffer_size, format, argptr);
      va_end(argptr);
    }

    printf("%s.\n", buffer);
}

You can, of course, change the memory management strategy to something different, like use a fixed local buffer of 512 bytes in the first call, and then use a temporary dynamically allocated buffer in the second call only if 512 proves to be insufficient. And so on...

AnT stands with Russia
  • 312,472
  • 42
  • 525
  • 765
  • `vsnprintf()` is probably a better choice given the fixed-size buffer in the question's code. – Andrew Henle Mar 30 '17 at 22:07
  • @AndrewHenle Are you saying there's a way to have an unfixed buffer size somehow because I'd much prefer that solution. – Hatefiend Mar 30 '17 at 22:10
  • @Hatefiend: Using standard tools, you can first do `int size = vsnprintf(NULL, 0,...`, (i.e. with `NULL` buffer). Such call will perform a "dry run" and calculate the exact required buffer size (don't forget about the null terminator). After that you can allocate buffer memory of appropriate size and call `vsnprintf` again, this time passing your freshly allocated buffer there. As you see, this requires two-pass processing. Decide for yourself, whether it is worth it in your case. – AnT stands with Russia Mar 30 '17 at 22:13
  • 1
    @Hatefiend In general, I prefer to do the first run of `vsnprintf()` with a local **non-static** buffer, then dynamically allocate a buffer only if that local buffer is too small. The one concern I have with the example in this answer is the use of a `static char *buffer;` The use of `static` variables makes the code much more efficient, but at the cost of multithread safety. If multiple threads make simultaneous calls to the `foo()` as currently written, there'd be serious problems. The best solution depends on how you'd use the function - you may not care about thread safety at all. – Andrew Henle Mar 30 '17 at 22:47
  • 1
    @AnT Why do you add one to `length` and check if it's greater than `buffer_length`? Shouldn't `length` and `buffer_length` be the same size since `vsnprintf` returns the amount of characters in the string? – Hatefiend Mar 30 '17 at 23:32
  • @Hatefiend: `buffer_length` should be greater than expected string length at least by 1 - to accomodate zero terminator character. Return value of `vsnprintf` does not include zero terminator into the count. For example, if `vsnprintf` returns 10, you need to allocate a buffer of length 11 (at least) to accomodate the string. And the second parameter of `vsnprintf` is buffer size (not string length), meaning that you'll have to pass 11 (not 10) into the subsequent call to `vsnprintf`. – AnT stands with Russia Mar 31 '17 at 00:52