2
\$\begingroup\$

When programming debug functions I am left with 2 options, either put them all within preprocessor blocks so that they don't exist at release runtime, or put them within an if block with a debug flag constant variable (determined at launch) to control their execution. I wanted to write an improved method where in release mode the end user doesn't have access to the functions, but with a special DLL they can use them.

I want to know two things:

  1. Is this more efficient than using a debug flag constant (determined at program launch) variable to control there execution. (e.g. will control flow prediction simply ignore all the calls to these debug functions)
  2. Is this is a good practice. To be precise, would this be frowned upon by developers or horrible for some other reason (like security).

The DLL contains an essentially empty DllMain, here are the important definitions:

#include <iostream>

extern "C" void __cdecl println(const char* str)
{
#ifdef GEN_DEBUG_FUNCS
    std::cout << str << std::endl;
#endif
}

extern "C" int __cdecl test()
{
#ifdef GEN_DEBUG_FUNCS
    return 1;
#else
    return 0;
#endif
}

And on the primary program side of things we have:

The header to hold the function ptrs:

typedef void(__cdecl *f_println)(const char*);
typedef int(__cdecl *f_test)();

#ifdef FPTR_INIT
  #define EXTERN
#else
  #define EXTERN extern
#endif

EXTERN f_println println;
EXTERN f_test test;

/* Used for testing the functions in a CPP file other than main */
void testFuncs();

The CPP file to test the function pointers:

#include "FunctionPtrs.h"
#include <iostream>

void testFuncs()
{
    println("Hello World");
    std::cout << "funci() returned " << test() << std::endl;
}

And finally the Main CPP file which contains the entry-point and function pointer loading:

#include <windows.h>
#include <iostream>
#define FPTR_INIT
#include "FunctionPtrs.h"

int importDeclarations(const bool debug);

int main(int argc, char* argv[])
{
    bool debug = false;
    for(int i = 0; i < argc; ++i)
    {
        if(strcmp(argv[i], "--debug") == 0)
        { debug = true; }
    }

    const int status = importDeclarations(debug);
    if(status) { return status; }

    testFuncs();

    system("pause");

    return EXIT_SUCCESS;
}

int importDeclarations(const bool debug)
{
    const char* path = debug ? "%PATH_TO_DLL%\\debug\\DebugDLL.dll" : 
                               "%PATH_TO_DLL%\\release\\DebugDLL.dll";

    const HINSTANCE hGetProcIDDLL = LoadLibrary(path);

    if(!hGetProcIDDLL)
    {
        std::cout << "Could not load the dynamic library." << std::endl;
        return EXIT_FAILURE;
    }

    println = (f_println)GetProcAddress(hGetProcIDDLL, "println");
    test = (f_test)GetProcAddress(hGetProcIDDLL, "test");

    if(!println || !test)
    {
        std::cout << "Could not locate function." << std::endl;
        return EXIT_FAILURE;
    }

    return 0;
}
\$\endgroup\$
1
  • \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$
    – Heslacher
    Commented Jan 3, 2018 at 13:04

1 Answer 1

1
\$\begingroup\$

To answer your question, this is a fine way to do what you're attempting to do. OS vendors will often do something similar where they ship the release versions of their frameworks to end users, and developers can sometimes download debug versions that do more bounds checking and output more verbose messages when given bogus inputs. This allows developers to run additional tests of their own apps to ensure they aren't giving bad values to the OS when calling it.

I also appreciate that now the code can be written inline without a bunch of #ifdef BLAH statements around it or even if (debug) {} statements. I would definitely make sure that calling the release versions which do nothing doesn't slow down any critical inner loops in your processing, though. It's not real likely, but can happen on occasion.

As for a review, I did notice a few really minor things:

You have this line of code in testFuncs():

std::cout << "funci() returned " << test() << std::endl;

What is funci()? Shouldn't the displayed text be "test() returned "? As I say, it's minor, but I was confused while reading it.

I'd also avoid putting an if statement on the same line as its body like you do here:

if(status) { return status; }

When debugging, you'd hit that line, and if status is non-zero the app will suddenly quit when you step over the line. I know if that happened to me, I wouldn't immediately know what had just happened the first time I saw it. Having it on the next line would cause me to step into the body and see that the value is bad. I could then examine what it was before the app quit and the state went away. As I said, very minor, but just thought I'd mention them.

\$\endgroup\$
1
  • \$\begingroup\$ funci was a mistake, the reference code I found had called the function funci, apparently I forgot to change that in the print statement. \$\endgroup\$
    – vandench
    Commented Jan 3, 2018 at 12:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.