Resolving a link - follow-up

Question

Similar piece of code to that I recently posted as:

Resolving a link

I have another piece of code which cannot be as easily extracted out into a method:

#define THROW_LAST_WINDOWS_ERROR()\
    WindowsApi::Exception::Throw(::GetLastError(), __FILE__,  __LINE__)

#define THROW_MANUAL_WINDOWS_ERROR(x)\
    WindowsApi::Exception::Throw(x, __FILE__,  __LINE__)

Process CreateNormalProcess(
    ProcessSnapshot *parent,
    const UNICODE_STRING& name, 
    const unsigned __int32 pid,
    const std::vector<ToolHelpThread>& threads
    )
{
    std::wstring nameStr(name.Buffer, name.Length/sizeof(wchar_t));
    std::wstring commandLine;
    std::wstring mainModulePath;
    std::wstring error;
    std::vector<Module> modules;
    try
    {
        using WindowsApi::Dll::NtDll;
        using WindowsApi::AutoArray;
        HANDLE hProc = OpenProcess(
            PROCESS_VM_READ | PROCESS_QUERY_INFORMATION, FALSE,
            pid);
        if (hProc == 0)
        {
            THROW_LAST_WINDOWS_ERROR();
        }

        //Populate the process environment block
        NtDll ntDll;
        PEB peb;
        AutoArray procInfoBuf = ntDll.NtQueryInformationProcess(
            ProcessBasicInformation,
            hProc,
            sizeof(PROCESS_BASIC_INFORMATION)
            );
        BOOL rpmError = ReadProcessMemory(
            hProc,
            procInfoBuf.GetAs<PROCESS_BASIC_INFORMATION>()->PebBaseAddress,
            &peb,
            sizeof(peb),
            0);
        if (rpmError == 0)
        {
            THROW_LAST_WINDOWS_ERROR();
        }

        RTL_USER_PROCESS_PARAMETERS procParameters;
        rpmError = ReadProcessMemory(hProc,
            peb.ProcessParameters,
            &procParameters,
            sizeof(procParameters),
            0);
        if (rpmError == 0)
        {
            THROW_LAST_WINDOWS_ERROR();
        }

        commandLine.assign(ReadRemoteUnicodeString(hProc,
            procParameters.CommandLine));
        mainModulePath.assign(ReadRemoteUnicodeString(hProc,
            procParameters.ImagePathName));

        PEB_LDR_DATA ldrData;
        rpmError = ReadProcessMemory(hProc,
            peb.Ldr,
            static_cast<void *>(&ldrData),
            sizeof(PEB_LDR_DATA),
            0);
        if (rpmError == 0)
        {
            THROW_LAST_WINDOWS_ERROR();
        }

        void * endPointer = static_cast<void *>
            (reinterpret_cast<char *>(peb.Ldr) + (reinterpret_cast<char *>(&ldrData.InLoadOrderModuleList) - reinterpret_cast<char *>(&ldrData)));
        void * currentListEntry = ldrData.InLoadOrderModuleList.Flink;
        while (currentListEntry != endPointer)
        {
            LDR_MODULE loaderModule;
            rpmError = ReadProcessMemory(
                hProc,
                currentListEntry,
                &loaderModule,
                sizeof(loaderModule),
                0);
            if (rpmError == 0)
            {
                THROW_LAST_WINDOWS_ERROR();
            }
            std::wstring moduleName = ReadRemoteUnicodeString(hProc, loaderModule.FullDllName);
            modules.push_back(Module(
                moduleName,
                loaderModule.BaseAddress,
                loaderModule.SizeOfImage
                ));
            currentListEntry = loaderModule.InLoadOrderModuleList.Flink;
        }

        CloseHandle(hProc);
    }
    catch (const ErrorAccessDeniedException&)
    {
        error.assign(L"ERROR: Could not access additional information because access "
            L"was denied while attempting to open the process. Are you admin? Do you "
            L"have SeDebugPrivilege?");
    }
    catch (const ErrorInvalidParameterException&)
    {
        error.assign(L"ERROR: The process terminated before additional information "
            L"could be extracted");
    }
    catch (const ErrorPartialCopyException&)
    {
        error.assign(L"ERROR: Couldn't copy a data structure from this process. Either "
            L"the process terminated before information extraction, or you are running "
            L"the 32 bit version of pevFind on a 64 bit machine.");
    }

    if (!error.empty())
    {
        if (nameStr.empty())
        {
            nameStr = error;
        }
        if (commandLine.empty())
        {
            commandLine = error;
        }
        if (mainModulePath.empty())
        {
            mainModulePath = error;
        }
    }

    return Process(
        parent,
        pid,
        nameStr, 
        commandLine,
        mainModulePath,
        threads,
        modules);
}

The problem here is twofold:

1. I can't extract out the duplicated if blocks into a method, because it's necessary that they embed the __FILE__ and __LINE__.

2. I can't put the conditional check into a macro, because the controlled method spans multiple lines:

   MYMACRO(
       Function(
       Call,
       Spanning,
       Multiple,
       Lines));
   

   doesn't seem to expand correctly.

Answer 1

The MYMACRO call should be fine.

#define D3DCALL(a) { auto __ = a; if (FAILED(__)) DXTrace(__FILE__, __LINE__, __, WIDEN(#a), TRUE); }
D3DCALL(D3DXCreateSphere(
    D3DDev.get(),
    radius,
    slices,
    slices,
    &retval->Mesh._Myptr,
    &retval->Adjacency._Myptr
));

I use this in my own source code all the time. The main way you get problems is if you try to conditionally compile within the macro call.

Answer 2

If the question is how to take something like this:

HANDLE hProc = OpenProcess(
            PROCESS_VM_READ | PROCESS_QUERY_INFORMATION, FALSE,
            pid);
        if (hProc == 0)
        {
            THROW_LAST_WINDOWS_ERROR();
        }

And reduce it to a one-liner, I usually use a verify macro:

template<class Eval> RetVal Verify(Eval eval, static const string& file, unsigned line)
{
  if( !eval )
    throw MyException(error_string, file, line);
  else 
    return eval;
}

#define verify(EVAL) (Verify(EVAL, __FILE__, __LINE__))

// ...

HANDLE hProc = verify( OpenProcess(PROCESS_VM_READ | PROCESS_QUERY_INFORMATION, FALSE, pid ));

Answer 3

This answer is not the best one, but you could create a new variable for each time you're assigning a value to rpmError and have one if statement where the last one is to check if any of them are equal to 0.

Note: This will not work if it is absolutely necessary that those errors are thrown before the next check.

Answer 4

Adapted from Charles' answer from your original post, would something like this work?

#define THROW_LAST_WINDOWS_ERROR(condition) ThrowOnFail(condition, __FILE__, __LINE__)

void ThrowOnFail(const BOOL condition, LPCSTR file_name, const int file_line)
{
  if (!condition)
    WindowsApi::Exception::Throw(::GetLastError(), file_name, file_line);
}

Then error handling would look something like this:

THROW_LAST_WINDOWS_ERROR(ReadProcessMemory(hProc,
                                           procInfoBuf.GetAs<PROCESS_BASIC_INFORMATION>()->PebBaseAddress,
                                           &peb,
                                           sizeof(peb),
                                           0));
THROW_LAST_WINDOWS_ERROR(ReadProcessMemory(hProc,
                                           peb.Ldr,
                                           static_cast<void *>(&ldrData),
                                           sizeof(PEB_LDR_DATA),
                                           0));
// ...
    while (currentListEntry != endPointer)
    {
        LDR_MODULE loaderModule;

        THROW_LAST_WINDOWS_ERROR(ReadProcessMemory(
            hProc,
            currentListEntry,
            &loaderModule,
            sizeof(loaderModule),
            0));

    // ...
    } 

Answer 5

You should also have a variant for explicitly testing return values. For example, CreateFile will return INVALID_HANDLE_VALUE which is !=0.

EDIT: Ah, I think I understand the question now. You want to know how to accomplish this:

int x= foo();
if (x==error) { blah(); }

in a macro.

The easiest way is to split the variable declaration and variable setting:

int x;
if ((x= foo())==error) { blah(); }

which you can then wrap in a macro:

#define MYMACRO(x) if ((x)==error) { blah(); }

which you can then use as:

int x;
MYMACRO(x= foo());