OutpostUniverse / op2ext

Outpost 2 extension module loader
1 stars 0 forks source link

OPMemory parameter consistency #274

Closed DanRStevens closed 4 years ago

DanRStevens commented 4 years ago

I noticed the parameter order for theses methods seems oddly inconsistent:

// OP2Memory.cpp
template <typename Function>
bool Op2MemEdit(std::uintptr_t destBaseAddr, std::size_t size, Function memoryEditFunction)

// OP2Memory.h/.cpp
bool Op2UnprotectMemory(std::uintptr_t destBaseAddr, std::size_t size);
bool Op2MemCopy(std::uintptr_t destBaseAddr, const void* sourceAddr, std::size_t size);
bool Op2MemSet(std::uintptr_t destBaseAddr, unsigned char value, std::size_t size);

I was thinking the std::size_t size parameter should be more closely and consistently paired with the std::uintptr_t destBaseAddr parameter that it corresponds to. The variable data parameter can perhaps be moved last.


Most methods take the same first parameter std::uintptr_t destBaseAddr:

bool Op2MemCopy(std::uintptr_t destBaseAddr, const void* sourceAddr, std::size_t size);
bool Op2MemSet(std::uintptr_t destBaseAddr, unsigned char value, std::size_t size);
bool Op2MemSetDword(std::uintptr_t destBaseAddr, std::size_t dword);
bool Op2MemSetDword(std::uintptr_t destBaseAddr, const void* dword);
bool Op2RelinkCall(std::uintptr_t callOffset, const void* newFunctionAddress);
bool Op2UnprotectMemory(std::uintptr_t destBaseAddr, std::size_t size);

There is however an inconsistency to the name for the Op2RelinkCall method. Perhaps this should be made to match? Or is the function specific enough that it should have a more specific parameter name? Another option might be to add documentation just before the function implementation in the .cpp file.