7

Typically, when I need to do debug output in Windows, I use the following C code segment:

#ifdef _DEBUG
#define DBGPRINT( kwszDebugFormatString, ... ) \
{ \
    wprintf_s( L"[%s:%d] ", __FUNCTIONW__, __LINE__ ); \
    wprintf_s( kwszDebugFormatString, __VA_ARGS__ ); \
}
#else
#define DBGPRINT( kwszDebugFormatString, ...) ;;
#endif

I'd like to recode this to use OutputDebugString which doesn't accept a format string. I consider statically allocating a small array on the stack (e.g., WCHAR wszBuf[100] = {0};) to be somewhat crude since it might consume significantly more or less memory than what is allocated and either truncate output or waste memory. I wrote the following code that addresses all of these concerns, but concerns me because the macro is a bit large.

#ifdef _DEBUG
#define DBGPRINT( kwszDebugFormatString, ... ) \
{ \
    INT iLineNumber = __LINE__; \
    FILE *fileNul = NULL; \
    INT cbFormatString = 0; \
    PWCHAR wszDebugString = NULL; \
    size_t st_Offset = 0; \
    \
    /* Determine the number of characters in the format string by writing to NUL. */\
    fopen_s( &fileNul, "nul", "w" ); \
    cbFormatString = fwprintf_s( fileNul, L"[%s:%d]", __FUNCTIONW__, iLineNumber ) * sizeof( WCHAR ); \
    cbFormatString += fwprintf_s( fileNul, kwszDebugFormatString, __VA_ARGS__ ) * sizeof( WCHAR ) + 2; \
    \
    /* Depending on the size of the format string, allocate space on the stack or the heap. */ \
    wszDebugString = (PWCHAR)_malloca( cbFormatString ); \
    \
    /* Populate the buffer with the contents of the format string. */ \
    StringCbPrintfW( wszDebugString, cbFormatString, L"[%s:%d]", __FUNCTIONW__, iLineNumber ); \
    StringCbLengthW( wszDebugString, cbFormatString, &st_Offset ); \
    StringCbPrintfW( &wszDebugString[st_Offset / sizeof(WCHAR)], cbFormatString - st_Offset, kwszDebugFormatString, __VA_ARGS__ ); \
    \
    OutputDebugStringW( wszDebugString ); \
    \
    _freea( wszDebugString ); \
    fclose( fileNul ); \
}
#else
#define DBGPRINT( kwszDebugFormatString, ... ) ;;
#endif

A few notes:

  • I used _malloca() and _freea() so that stack allocations could be used if the output should be small enough.
  • I'm not aware of any way to get the correct size of a fully expanded format string. The best solution I could come up with was to dump it to NUL and calculate the correct sizes from there.
  • I used a macro because I don't believe there is a simple way to achieve the same result when using a function.

My question is, quite simply, would this macro be considered bad practice for one reason or another (particularly for size or inefficiency)? If so, what alternatives should I be considering?

Solution

In case anyone else is wondering, I used the suggestions from the comments and the selected answer and came up with the following code. Thank you so much to everyone who commented or answered - feel free to add more if you have a clever way of doing this that you want to share!

#ifdef _DEBUG
#define DBGPRINT(kwszDebugFormatString, ...) _DBGPRINT(__FUNCTIONW__, __LINE__, kwszDebugFormatString, __VA_ARGS__)

VOID _DBGPRINT( LPCWSTR kwszFunction, INT iLineNumber, LPCWSTR kwszDebugFormatString, ... ) \
{
    INT cbFormatString = 0;
    va_list args;
    PWCHAR wszDebugString = NULL;
    size_t st_Offset = 0;

    va_start( args, kwszDebugFormatString );

    cbFormatString = _scwprintf( L"[%s:%d] ", kwszFunction, iLineNumber ) * sizeof( WCHAR );
    cbFormatString += _vscwprintf( kwszDebugFormatString, args ) * sizeof( WCHAR ) + 2;

    /* Depending on the size of the format string, allocate space on the stack or the heap. */
    wszDebugString = (PWCHAR)_malloca( cbFormatString );

    /* Populate the buffer with the contents of the format string. */
    StringCbPrintfW( wszDebugString, cbFormatString, L"[%s:%d] ", kwszFunction, iLineNumber );
    StringCbLengthW( wszDebugString, cbFormatString, &st_Offset );
    StringCbVPrintfW( &wszDebugString[st_Offset / sizeof(WCHAR)], cbFormatString - st_Offset, kwszDebugFormatString, args );

    OutputDebugStringW( wszDebugString );

    _freea( wszDebugString );
    va_end( args );
}
#else
#define DBGPRINT( kwszDebugFormatString, ... ) ;;
#endif
Kevin
  • 83
  • 1
  • 5
  • 1
    This question is opinion based because even though I don't think they are bad practice, if you really need one such huge macro, someone might say that they are bad practice, so voting to close as "*Primarily Opinion Based*". – Iharob Al Asimi Mar 14 '15 at 13:53
  • 2
    Why not use a function? – alk Mar 14 '15 at 14:00
  • And why not use `vsprintf()` to create the string prior to logging it? – alk Mar 14 '15 at 14:02
  • @iharob While code style certainly has elements for opinions, there are many standards and considerations to take into account. Not only that, there could be various reason that one might not want to use this method that I am unaware of. – Kevin Mar 14 '15 at 14:02
  • 1
    Which standard do you want us to apply? Here's my standard: *macros are bad practise.* – David Heffernan Mar 14 '15 at 14:05
  • @alk - I mentioned in the third bullet why I cannot use a function. The `__FUNCTIONW__` and `__LINE__` macros would produce invalid results. Using `vsprintf` would tie back into the function question, but also would force me to statically allocate an array. – Kevin Mar 14 '15 at 14:06
  • @DavidHeffernan - I'll edit my original question so that it asks for more of a code critique or best practices instead of style. Additionally, with respect to your statement, why make a blanket statement? When used properly preprocess macros can be very powerful and can achieve things that may not otherwise be possible. Do you have an alternative to this method that preserves the desired functionality? – Kevin Mar 14 '15 at 14:09
  • Is there any particular reason why my question deserved a down vote? Is anyone else aware of a similar question, because I wasn't able to find any answers that quite fit the bill before this. – Kevin Mar 14 '15 at 14:22
  • 1
    @Jongware - Thanks. I rephrased it. I hope that is more appropriate. – Kevin Mar 14 '15 at 15:02
  • 1
    @Kevin [Microsoft's implementation of the C runtime contains a series of functions that will give you the size of a formatted string.](https://msdn.microsoft.com/en-us/library/t32cf9tb.aspx) Unfortunately, you will need to use `va_copy()` (which is a rather recent addition to Microsoft's headers) or two `va_start()` calls (which I don't know if it is legal C). `FormatMessage()` can also give you the size (and can even allocate a buffer) but using it with printf-style formats is a bit more work. (If you ever go to Unix, the equivalent is `asprintf()`; also nonstandard but de facto everywhere.) – andlabs Mar 14 '15 at 15:03
  • @andlabs - Oh wow - thank you so much for those API calls. I was combing the library and must have just missed them. Like others have suggested, I am using a function now, but eliminating the call to open a `NUL` pipe is nice (even if `_scprintf` ends up doing that under the hood). I did look at using `FormatMessage()` but, as you said, getting it to fit a printf-style message is a little crazy. Thanks for the Unix tip! – Kevin Mar 14 '15 at 15:51

2 Answers2

12

It would be much simpler and less error prone if you'd put all the code into a normal varargs function and then called that in your macro, similar to this:

void dbgprint(const wchar_t *func, int line, const wchar_t *fmt, ...) {
   // Fomat the string, maybe with vsprintf, log it, etc.
}

#define DBGPRINT(fmt, ...) dbgprint(__WFUNCTION__, __LINE__, fmt, __VA_ARGS__)
sth
  • 222,467
  • 53
  • 283
  • 367
  • Oh very interesting. You managed to preserve the function and line number while using a function instead of a pure macro. Thank you so much! – Kevin Mar 14 '15 at 14:12
  • @Kevin: "*... use a function?*" :-) – alk Mar 14 '15 at 14:14
  • 2
    @alk - Well what I mean to say is when you suggested that, I thought you were referring to something similar to `#define void foo(){}` but I never thought of defining a (for lack of a better term) 'macro function' that simply maps to the actual function while passing in the parameters that are position dependent. It's these subtle clever things in C that I love finding out :-) – Kevin Mar 14 '15 at 14:19
12

I'm currently using this way. Simple and fast. Absurdly simple :)

#define MY_PRINTF(...) {char cad[512]; sprintf(cad, __VA_ARGS__);  OutputDebugString(cad);}
ffelagund
  • 121
  • 1
  • 4
  • While this code is simple and fast, it's also vulnerable to a buffer overflow, so I'd recommend using a safe function like `snprintf` or `StringCchPrintf` instead of `sprintf` – labmonkey398 Aug 24 '23 at 18:50