Get module entry by name from external program via WinAPI

The name of the pictureThe name of the pictureThe name of the pictureClash Royale CLAN TAG#URR8PPP





.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;







up vote
4
down vote

favorite
1












I'm asking myself if there's any better/shorter way of getting a module entry by its name from an external process.



This is the code I have so far:



MODULEENTRY32 GetModule(const char* ModuleName) 

HANDLE Module = CreateToolhelp32Snapshot(TH32CS_SNAPMODULE, pid);
MODULEENTRY32 Entry;
Entry.dwSize = sizeof(Entry);

WCHAR *ModuleNameChar;
int Chars = MultiByteToWideChar(CP_ACP, 0, ModuleName, -1, NULL, 0);
ModuleNameChar = new WCHAR[Chars];
MultiByteToWideChar(CP_ACP, 0, ModuleName, -1, (LPWSTR)ModuleNameChar, Chars);

while (Module32Next(Module, &Entry))
if (!wcscmp((wchar_t*)Entry.szModule, ModuleNameChar))
CloseHandle(Module);
return Entry;



/*
Return the module base 0x0 if we don't find any module
*/
CloseHandle(Module);
Entry.modBaseAddr = 0x0;
return Entry;



It should work but it's quite complicated. Is there any useful function or method I can make use of to simplify this whole thing?







share|improve this question





















  • I'd say this is pretty concise for WinAPI code. When I wrote a similar function once it turned out longer.
    – yuri
    Aug 1 at 17:55
















up vote
4
down vote

favorite
1












I'm asking myself if there's any better/shorter way of getting a module entry by its name from an external process.



This is the code I have so far:



MODULEENTRY32 GetModule(const char* ModuleName) 

HANDLE Module = CreateToolhelp32Snapshot(TH32CS_SNAPMODULE, pid);
MODULEENTRY32 Entry;
Entry.dwSize = sizeof(Entry);

WCHAR *ModuleNameChar;
int Chars = MultiByteToWideChar(CP_ACP, 0, ModuleName, -1, NULL, 0);
ModuleNameChar = new WCHAR[Chars];
MultiByteToWideChar(CP_ACP, 0, ModuleName, -1, (LPWSTR)ModuleNameChar, Chars);

while (Module32Next(Module, &Entry))
if (!wcscmp((wchar_t*)Entry.szModule, ModuleNameChar))
CloseHandle(Module);
return Entry;



/*
Return the module base 0x0 if we don't find any module
*/
CloseHandle(Module);
Entry.modBaseAddr = 0x0;
return Entry;



It should work but it's quite complicated. Is there any useful function or method I can make use of to simplify this whole thing?







share|improve this question





















  • I'd say this is pretty concise for WinAPI code. When I wrote a similar function once it turned out longer.
    – yuri
    Aug 1 at 17:55












up vote
4
down vote

favorite
1









up vote
4
down vote

favorite
1






1





I'm asking myself if there's any better/shorter way of getting a module entry by its name from an external process.



This is the code I have so far:



MODULEENTRY32 GetModule(const char* ModuleName) 

HANDLE Module = CreateToolhelp32Snapshot(TH32CS_SNAPMODULE, pid);
MODULEENTRY32 Entry;
Entry.dwSize = sizeof(Entry);

WCHAR *ModuleNameChar;
int Chars = MultiByteToWideChar(CP_ACP, 0, ModuleName, -1, NULL, 0);
ModuleNameChar = new WCHAR[Chars];
MultiByteToWideChar(CP_ACP, 0, ModuleName, -1, (LPWSTR)ModuleNameChar, Chars);

while (Module32Next(Module, &Entry))
if (!wcscmp((wchar_t*)Entry.szModule, ModuleNameChar))
CloseHandle(Module);
return Entry;



/*
Return the module base 0x0 if we don't find any module
*/
CloseHandle(Module);
Entry.modBaseAddr = 0x0;
return Entry;



It should work but it's quite complicated. Is there any useful function or method I can make use of to simplify this whole thing?







share|improve this question













I'm asking myself if there's any better/shorter way of getting a module entry by its name from an external process.



This is the code I have so far:



MODULEENTRY32 GetModule(const char* ModuleName) 

HANDLE Module = CreateToolhelp32Snapshot(TH32CS_SNAPMODULE, pid);
MODULEENTRY32 Entry;
Entry.dwSize = sizeof(Entry);

WCHAR *ModuleNameChar;
int Chars = MultiByteToWideChar(CP_ACP, 0, ModuleName, -1, NULL, 0);
ModuleNameChar = new WCHAR[Chars];
MultiByteToWideChar(CP_ACP, 0, ModuleName, -1, (LPWSTR)ModuleNameChar, Chars);

while (Module32Next(Module, &Entry))
if (!wcscmp((wchar_t*)Entry.szModule, ModuleNameChar))
CloseHandle(Module);
return Entry;



/*
Return the module base 0x0 if we don't find any module
*/
CloseHandle(Module);
Entry.modBaseAddr = 0x0;
return Entry;



It should work but it's quite complicated. Is there any useful function or method I can make use of to simplify this whole thing?









share|improve this question












share|improve this question




share|improve this question








edited Aug 1 at 6:27









yuri

3,3872832




3,3872832









asked Jul 31 at 23:03









Intence

211




211











  • I'd say this is pretty concise for WinAPI code. When I wrote a similar function once it turned out longer.
    – yuri
    Aug 1 at 17:55
















  • I'd say this is pretty concise for WinAPI code. When I wrote a similar function once it turned out longer.
    – yuri
    Aug 1 at 17:55















I'd say this is pretty concise for WinAPI code. When I wrote a similar function once it turned out longer.
– yuri
Aug 1 at 17:55




I'd say this is pretty concise for WinAPI code. When I wrote a similar function once it turned out longer.
– yuri
Aug 1 at 17:55










2 Answers
2






active

oldest

votes

















up vote
1
down vote













Unfortunately, I'm not sure of any way of being more concise.



There are a few changes you could make, but they're mostly optional.




  1. Check the return value of CreateToolhelp32Snapshot(). The documentation says the following:




    Includes all modules of the process specified in th32ProcessID in the
    snapshot. To enumerate the modules, see Module32First. If the function
    fails with ERROR_BAD_LENGTH, retry the function until it succeeds.




    I use CreateToolhelp32Snapshot() in an application that runs 24/7 and I know for a fact that I get the ERROR_BAD_LENGTH error here and there. In my case, I cannot afford false negatives, so I have to put this function in a loop. If you are okay with having false-negatives occasionally, then I wouldn't worry too much about it.




  2. I don't know if you're showing us the full code for this function, but you have a memory leak here:



    ModuleNameChar = new WCHAR[Chars];



    I don't see where you are deleting this buffer. I personally would use an std::vector<WCHAR> as a buffer, give it the correct size, and pass the address of the first element to MultiByteToWideChar().



  3. If you use HANDLEs a lot, consider wrapping them in an RAII container. That way, you don't have to worry about closing them manually. This will also make your code exception-safe if you ever add any functions that can throw (you actually already have one that can throw, the new call).


  4. I noticed you're not calling ::Module32First(). You probably don't need it, but I always have it anyway.


  5. This is a personal preference, but I think your upper-case variable names look absolutely horrendous. Please use snake-case or camel-case for variable names.


There's really nothing helpful to say unfortunately, the Windows API can be quite cumbersome.






share|improve this answer























  • If the function fails with ERROR_BAD_LENGTH, retry the function until it succeeds. - this is of course documentation bug. you can note that CreateToolhelp32Snapshot not take any buffer size as input parameter. as result we can not increment buffer size before calling this api again. without this - call it again(with same parameters) absolute senseless - it again fail. this is how i say strange documentation bug
    – RbMm
    13 hours ago










  • @RbMm I have gotten this error before on real-life production servers. This isn't something I'm pulling out of my ass. This is definitely not a "pure" function, so you can get different results even with the same input.
    – ncalmbeblpaicr0011
    11 hours ago










  • you can reproduce this ? i absolute sure that this is impossible even from generic point of view (i already not say that i know how this api internal working). simply because caller not control/pass any buffer size. and callee can not know - are this is first, or say second call. which buffer size used before. try set yourself on place who write this api - how in case ERROR_BAD_LENGTH retry call can help ? if function in some internal place got this error - it must not pass it to caller, but retry some internal call with bigger buffer. this like say - if you got access denied - try again
    – RbMm
    10 hours ago


















up vote
1
down vote













more simply api, way not exist.



1.) wrong return type - MODULEENTRY32. really impossible return structure, which not fit generic register size. when compiler view T fn(..) * it silently change this to void fn(T* t, ...) * and in your code Entry will be copied, what really not need. you must not try return MODULEENTRY32 but pass pointer to it as function argument.



2.) you lost error value. the GetModule can fail by different reasons - can not exist process with such id (STATUS_INVALID_CID), you can fail open target process (STATUS_ACCESS_DENIED), module can simply not exist in process. without returned error value - you can not know reason why function fail. really the best practice always return win32 error code (or NTSTATUS) from function.



3.) why you use module name as ansi string ? this is always bad and error design. use unicode name format. and you will be not need convert it (i already not say that if module name (this is rarely and never for system modules) use non english chars - it can not have correct ansi name at all(and depend from system locale - you encode it on self comp in one locale, on another comp - can be another locale. but ansi name depend from it).



4.) so signature of function must be ULONG GetModule(DWORD th32ProcessID, MODULEENTRY32* Entry, PCWSTR szModule)



5.) use wcscmp is wrong here. we need case insensitive compare. - _wcsicmp



6.) Return the module base 0x0 if we don't find any module - how i say - we need return appropriate error code in this case, instead set base address at 0 (we can left it undefined). client code must check not base address for 0, but error code for 0 (NOERROR).



7.) not forget about 32-64 bit issues here (are your and target process have some bit-ness)



so code can look like:



ULONG GetModule(DWORD th32ProcessID, MODULEENTRY32* Entry, PCWSTR szModule)

HANDLE hSnapshot = CreateToolhelp32Snapshot(TH32CS_SNAPMODULE, th32ProcessID);

if (hSnapshot != INVALID_HANDLE_VALUE)

ULONG dwError = ERROR_NOT_FOUND;

Entry->dwSize = sizeof(MODULEENTRY32);

if (Module32FirstW(hSnapshot, Entry))

do

if (!_wcsicmp(Entry->szModule, szModule))

dwError = NOERROR;
break;

while (Module32NextW(hSnapshot, Entry));


CloseHandle(hSnapshot);

return dwError;


// return RtlGetLastNtStatus();
// this is much more better compare GetLastError();
return GetLastError();



and usage:



MODULEENTRY32 Entry;
if (ULONG dwError = GetModule(*, &Entry, L"*"))

// error, dont touch Entry

else

//ok, use Entry






share|improve this answer





















  • +1 You're answer makes some good points, but I disagree with #1. I believe many modern compilers can do copy elision and return value optimization, so passing a pointer or reference as an output is kind of old fashioned.
    – ncalmbeblpaicr0011
    11 hours ago










  • @ncalmbeblpaicr0011 - may be in some case compiler and can internal optimize this for avoid extra copy. but i think programmer must understand that internally anyway to function will be pass pointer to caller allocated structure. are compiler can direct use this pointer instead local copy in function (MODULEENTRY32 Entry) is question. anyway i personal think that this is very bad style. and good style - always return error code from function (as result we also need return value for error reserve).
    – RbMm
    10 hours ago











Your Answer




StackExchange.ifUsing("editor", function ()
return StackExchange.using("mathjaxEditing", function ()
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
);
);
, "mathjax-editing");

StackExchange.ifUsing("editor", function ()
StackExchange.using("externalEditor", function ()
StackExchange.using("snippets", function ()
StackExchange.snippets.init();
);
);
, "code-snippets");

StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "196"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);

StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);

else
createEditor();

);

function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
convertImagesToLinks: false,
noModals: false,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);



);








 

draft saved


draft discarded


















StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f200698%2fget-module-entry-by-name-from-external-program-via-winapi%23new-answer', 'question_page');

);

Post as a guest






























2 Answers
2






active

oldest

votes








2 Answers
2






active

oldest

votes









active

oldest

votes






active

oldest

votes








up vote
1
down vote













Unfortunately, I'm not sure of any way of being more concise.



There are a few changes you could make, but they're mostly optional.




  1. Check the return value of CreateToolhelp32Snapshot(). The documentation says the following:




    Includes all modules of the process specified in th32ProcessID in the
    snapshot. To enumerate the modules, see Module32First. If the function
    fails with ERROR_BAD_LENGTH, retry the function until it succeeds.




    I use CreateToolhelp32Snapshot() in an application that runs 24/7 and I know for a fact that I get the ERROR_BAD_LENGTH error here and there. In my case, I cannot afford false negatives, so I have to put this function in a loop. If you are okay with having false-negatives occasionally, then I wouldn't worry too much about it.




  2. I don't know if you're showing us the full code for this function, but you have a memory leak here:



    ModuleNameChar = new WCHAR[Chars];



    I don't see where you are deleting this buffer. I personally would use an std::vector<WCHAR> as a buffer, give it the correct size, and pass the address of the first element to MultiByteToWideChar().



  3. If you use HANDLEs a lot, consider wrapping them in an RAII container. That way, you don't have to worry about closing them manually. This will also make your code exception-safe if you ever add any functions that can throw (you actually already have one that can throw, the new call).


  4. I noticed you're not calling ::Module32First(). You probably don't need it, but I always have it anyway.


  5. This is a personal preference, but I think your upper-case variable names look absolutely horrendous. Please use snake-case or camel-case for variable names.


There's really nothing helpful to say unfortunately, the Windows API can be quite cumbersome.






share|improve this answer























  • If the function fails with ERROR_BAD_LENGTH, retry the function until it succeeds. - this is of course documentation bug. you can note that CreateToolhelp32Snapshot not take any buffer size as input parameter. as result we can not increment buffer size before calling this api again. without this - call it again(with same parameters) absolute senseless - it again fail. this is how i say strange documentation bug
    – RbMm
    13 hours ago










  • @RbMm I have gotten this error before on real-life production servers. This isn't something I'm pulling out of my ass. This is definitely not a "pure" function, so you can get different results even with the same input.
    – ncalmbeblpaicr0011
    11 hours ago










  • you can reproduce this ? i absolute sure that this is impossible even from generic point of view (i already not say that i know how this api internal working). simply because caller not control/pass any buffer size. and callee can not know - are this is first, or say second call. which buffer size used before. try set yourself on place who write this api - how in case ERROR_BAD_LENGTH retry call can help ? if function in some internal place got this error - it must not pass it to caller, but retry some internal call with bigger buffer. this like say - if you got access denied - try again
    – RbMm
    10 hours ago















up vote
1
down vote













Unfortunately, I'm not sure of any way of being more concise.



There are a few changes you could make, but they're mostly optional.




  1. Check the return value of CreateToolhelp32Snapshot(). The documentation says the following:




    Includes all modules of the process specified in th32ProcessID in the
    snapshot. To enumerate the modules, see Module32First. If the function
    fails with ERROR_BAD_LENGTH, retry the function until it succeeds.




    I use CreateToolhelp32Snapshot() in an application that runs 24/7 and I know for a fact that I get the ERROR_BAD_LENGTH error here and there. In my case, I cannot afford false negatives, so I have to put this function in a loop. If you are okay with having false-negatives occasionally, then I wouldn't worry too much about it.




  2. I don't know if you're showing us the full code for this function, but you have a memory leak here:



    ModuleNameChar = new WCHAR[Chars];



    I don't see where you are deleting this buffer. I personally would use an std::vector<WCHAR> as a buffer, give it the correct size, and pass the address of the first element to MultiByteToWideChar().



  3. If you use HANDLEs a lot, consider wrapping them in an RAII container. That way, you don't have to worry about closing them manually. This will also make your code exception-safe if you ever add any functions that can throw (you actually already have one that can throw, the new call).


  4. I noticed you're not calling ::Module32First(). You probably don't need it, but I always have it anyway.


  5. This is a personal preference, but I think your upper-case variable names look absolutely horrendous. Please use snake-case or camel-case for variable names.


There's really nothing helpful to say unfortunately, the Windows API can be quite cumbersome.






share|improve this answer























  • If the function fails with ERROR_BAD_LENGTH, retry the function until it succeeds. - this is of course documentation bug. you can note that CreateToolhelp32Snapshot not take any buffer size as input parameter. as result we can not increment buffer size before calling this api again. without this - call it again(with same parameters) absolute senseless - it again fail. this is how i say strange documentation bug
    – RbMm
    13 hours ago










  • @RbMm I have gotten this error before on real-life production servers. This isn't something I'm pulling out of my ass. This is definitely not a "pure" function, so you can get different results even with the same input.
    – ncalmbeblpaicr0011
    11 hours ago










  • you can reproduce this ? i absolute sure that this is impossible even from generic point of view (i already not say that i know how this api internal working). simply because caller not control/pass any buffer size. and callee can not know - are this is first, or say second call. which buffer size used before. try set yourself on place who write this api - how in case ERROR_BAD_LENGTH retry call can help ? if function in some internal place got this error - it must not pass it to caller, but retry some internal call with bigger buffer. this like say - if you got access denied - try again
    – RbMm
    10 hours ago













up vote
1
down vote










up vote
1
down vote









Unfortunately, I'm not sure of any way of being more concise.



There are a few changes you could make, but they're mostly optional.




  1. Check the return value of CreateToolhelp32Snapshot(). The documentation says the following:




    Includes all modules of the process specified in th32ProcessID in the
    snapshot. To enumerate the modules, see Module32First. If the function
    fails with ERROR_BAD_LENGTH, retry the function until it succeeds.




    I use CreateToolhelp32Snapshot() in an application that runs 24/7 and I know for a fact that I get the ERROR_BAD_LENGTH error here and there. In my case, I cannot afford false negatives, so I have to put this function in a loop. If you are okay with having false-negatives occasionally, then I wouldn't worry too much about it.




  2. I don't know if you're showing us the full code for this function, but you have a memory leak here:



    ModuleNameChar = new WCHAR[Chars];



    I don't see where you are deleting this buffer. I personally would use an std::vector<WCHAR> as a buffer, give it the correct size, and pass the address of the first element to MultiByteToWideChar().



  3. If you use HANDLEs a lot, consider wrapping them in an RAII container. That way, you don't have to worry about closing them manually. This will also make your code exception-safe if you ever add any functions that can throw (you actually already have one that can throw, the new call).


  4. I noticed you're not calling ::Module32First(). You probably don't need it, but I always have it anyway.


  5. This is a personal preference, but I think your upper-case variable names look absolutely horrendous. Please use snake-case or camel-case for variable names.


There's really nothing helpful to say unfortunately, the Windows API can be quite cumbersome.






share|improve this answer















Unfortunately, I'm not sure of any way of being more concise.



There are a few changes you could make, but they're mostly optional.




  1. Check the return value of CreateToolhelp32Snapshot(). The documentation says the following:




    Includes all modules of the process specified in th32ProcessID in the
    snapshot. To enumerate the modules, see Module32First. If the function
    fails with ERROR_BAD_LENGTH, retry the function until it succeeds.




    I use CreateToolhelp32Snapshot() in an application that runs 24/7 and I know for a fact that I get the ERROR_BAD_LENGTH error here and there. In my case, I cannot afford false negatives, so I have to put this function in a loop. If you are okay with having false-negatives occasionally, then I wouldn't worry too much about it.




  2. I don't know if you're showing us the full code for this function, but you have a memory leak here:



    ModuleNameChar = new WCHAR[Chars];



    I don't see where you are deleting this buffer. I personally would use an std::vector<WCHAR> as a buffer, give it the correct size, and pass the address of the first element to MultiByteToWideChar().



  3. If you use HANDLEs a lot, consider wrapping them in an RAII container. That way, you don't have to worry about closing them manually. This will also make your code exception-safe if you ever add any functions that can throw (you actually already have one that can throw, the new call).


  4. I noticed you're not calling ::Module32First(). You probably don't need it, but I always have it anyway.


  5. This is a personal preference, but I think your upper-case variable names look absolutely horrendous. Please use snake-case or camel-case for variable names.


There's really nothing helpful to say unfortunately, the Windows API can be quite cumbersome.







share|improve this answer















share|improve this answer



share|improve this answer








edited Aug 2 at 15:44


























answered Aug 2 at 15:06









ncalmbeblpaicr0011

378210




378210











  • If the function fails with ERROR_BAD_LENGTH, retry the function until it succeeds. - this is of course documentation bug. you can note that CreateToolhelp32Snapshot not take any buffer size as input parameter. as result we can not increment buffer size before calling this api again. without this - call it again(with same parameters) absolute senseless - it again fail. this is how i say strange documentation bug
    – RbMm
    13 hours ago










  • @RbMm I have gotten this error before on real-life production servers. This isn't something I'm pulling out of my ass. This is definitely not a "pure" function, so you can get different results even with the same input.
    – ncalmbeblpaicr0011
    11 hours ago










  • you can reproduce this ? i absolute sure that this is impossible even from generic point of view (i already not say that i know how this api internal working). simply because caller not control/pass any buffer size. and callee can not know - are this is first, or say second call. which buffer size used before. try set yourself on place who write this api - how in case ERROR_BAD_LENGTH retry call can help ? if function in some internal place got this error - it must not pass it to caller, but retry some internal call with bigger buffer. this like say - if you got access denied - try again
    – RbMm
    10 hours ago

















  • If the function fails with ERROR_BAD_LENGTH, retry the function until it succeeds. - this is of course documentation bug. you can note that CreateToolhelp32Snapshot not take any buffer size as input parameter. as result we can not increment buffer size before calling this api again. without this - call it again(with same parameters) absolute senseless - it again fail. this is how i say strange documentation bug
    – RbMm
    13 hours ago










  • @RbMm I have gotten this error before on real-life production servers. This isn't something I'm pulling out of my ass. This is definitely not a "pure" function, so you can get different results even with the same input.
    – ncalmbeblpaicr0011
    11 hours ago










  • you can reproduce this ? i absolute sure that this is impossible even from generic point of view (i already not say that i know how this api internal working). simply because caller not control/pass any buffer size. and callee can not know - are this is first, or say second call. which buffer size used before. try set yourself on place who write this api - how in case ERROR_BAD_LENGTH retry call can help ? if function in some internal place got this error - it must not pass it to caller, but retry some internal call with bigger buffer. this like say - if you got access denied - try again
    – RbMm
    10 hours ago
















If the function fails with ERROR_BAD_LENGTH, retry the function until it succeeds. - this is of course documentation bug. you can note that CreateToolhelp32Snapshot not take any buffer size as input parameter. as result we can not increment buffer size before calling this api again. without this - call it again(with same parameters) absolute senseless - it again fail. this is how i say strange documentation bug
– RbMm
13 hours ago




If the function fails with ERROR_BAD_LENGTH, retry the function until it succeeds. - this is of course documentation bug. you can note that CreateToolhelp32Snapshot not take any buffer size as input parameter. as result we can not increment buffer size before calling this api again. without this - call it again(with same parameters) absolute senseless - it again fail. this is how i say strange documentation bug
– RbMm
13 hours ago












@RbMm I have gotten this error before on real-life production servers. This isn't something I'm pulling out of my ass. This is definitely not a "pure" function, so you can get different results even with the same input.
– ncalmbeblpaicr0011
11 hours ago




@RbMm I have gotten this error before on real-life production servers. This isn't something I'm pulling out of my ass. This is definitely not a "pure" function, so you can get different results even with the same input.
– ncalmbeblpaicr0011
11 hours ago












you can reproduce this ? i absolute sure that this is impossible even from generic point of view (i already not say that i know how this api internal working). simply because caller not control/pass any buffer size. and callee can not know - are this is first, or say second call. which buffer size used before. try set yourself on place who write this api - how in case ERROR_BAD_LENGTH retry call can help ? if function in some internal place got this error - it must not pass it to caller, but retry some internal call with bigger buffer. this like say - if you got access denied - try again
– RbMm
10 hours ago





you can reproduce this ? i absolute sure that this is impossible even from generic point of view (i already not say that i know how this api internal working). simply because caller not control/pass any buffer size. and callee can not know - are this is first, or say second call. which buffer size used before. try set yourself on place who write this api - how in case ERROR_BAD_LENGTH retry call can help ? if function in some internal place got this error - it must not pass it to caller, but retry some internal call with bigger buffer. this like say - if you got access denied - try again
– RbMm
10 hours ago













up vote
1
down vote













more simply api, way not exist.



1.) wrong return type - MODULEENTRY32. really impossible return structure, which not fit generic register size. when compiler view T fn(..) * it silently change this to void fn(T* t, ...) * and in your code Entry will be copied, what really not need. you must not try return MODULEENTRY32 but pass pointer to it as function argument.



2.) you lost error value. the GetModule can fail by different reasons - can not exist process with such id (STATUS_INVALID_CID), you can fail open target process (STATUS_ACCESS_DENIED), module can simply not exist in process. without returned error value - you can not know reason why function fail. really the best practice always return win32 error code (or NTSTATUS) from function.



3.) why you use module name as ansi string ? this is always bad and error design. use unicode name format. and you will be not need convert it (i already not say that if module name (this is rarely and never for system modules) use non english chars - it can not have correct ansi name at all(and depend from system locale - you encode it on self comp in one locale, on another comp - can be another locale. but ansi name depend from it).



4.) so signature of function must be ULONG GetModule(DWORD th32ProcessID, MODULEENTRY32* Entry, PCWSTR szModule)



5.) use wcscmp is wrong here. we need case insensitive compare. - _wcsicmp



6.) Return the module base 0x0 if we don't find any module - how i say - we need return appropriate error code in this case, instead set base address at 0 (we can left it undefined). client code must check not base address for 0, but error code for 0 (NOERROR).



7.) not forget about 32-64 bit issues here (are your and target process have some bit-ness)



so code can look like:



ULONG GetModule(DWORD th32ProcessID, MODULEENTRY32* Entry, PCWSTR szModule)

HANDLE hSnapshot = CreateToolhelp32Snapshot(TH32CS_SNAPMODULE, th32ProcessID);

if (hSnapshot != INVALID_HANDLE_VALUE)

ULONG dwError = ERROR_NOT_FOUND;

Entry->dwSize = sizeof(MODULEENTRY32);

if (Module32FirstW(hSnapshot, Entry))

do

if (!_wcsicmp(Entry->szModule, szModule))

dwError = NOERROR;
break;

while (Module32NextW(hSnapshot, Entry));


CloseHandle(hSnapshot);

return dwError;


// return RtlGetLastNtStatus();
// this is much more better compare GetLastError();
return GetLastError();



and usage:



MODULEENTRY32 Entry;
if (ULONG dwError = GetModule(*, &Entry, L"*"))

// error, dont touch Entry

else

//ok, use Entry






share|improve this answer





















  • +1 You're answer makes some good points, but I disagree with #1. I believe many modern compilers can do copy elision and return value optimization, so passing a pointer or reference as an output is kind of old fashioned.
    – ncalmbeblpaicr0011
    11 hours ago










  • @ncalmbeblpaicr0011 - may be in some case compiler and can internal optimize this for avoid extra copy. but i think programmer must understand that internally anyway to function will be pass pointer to caller allocated structure. are compiler can direct use this pointer instead local copy in function (MODULEENTRY32 Entry) is question. anyway i personal think that this is very bad style. and good style - always return error code from function (as result we also need return value for error reserve).
    – RbMm
    10 hours ago















up vote
1
down vote













more simply api, way not exist.



1.) wrong return type - MODULEENTRY32. really impossible return structure, which not fit generic register size. when compiler view T fn(..) * it silently change this to void fn(T* t, ...) * and in your code Entry will be copied, what really not need. you must not try return MODULEENTRY32 but pass pointer to it as function argument.



2.) you lost error value. the GetModule can fail by different reasons - can not exist process with such id (STATUS_INVALID_CID), you can fail open target process (STATUS_ACCESS_DENIED), module can simply not exist in process. without returned error value - you can not know reason why function fail. really the best practice always return win32 error code (or NTSTATUS) from function.



3.) why you use module name as ansi string ? this is always bad and error design. use unicode name format. and you will be not need convert it (i already not say that if module name (this is rarely and never for system modules) use non english chars - it can not have correct ansi name at all(and depend from system locale - you encode it on self comp in one locale, on another comp - can be another locale. but ansi name depend from it).



4.) so signature of function must be ULONG GetModule(DWORD th32ProcessID, MODULEENTRY32* Entry, PCWSTR szModule)



5.) use wcscmp is wrong here. we need case insensitive compare. - _wcsicmp



6.) Return the module base 0x0 if we don't find any module - how i say - we need return appropriate error code in this case, instead set base address at 0 (we can left it undefined). client code must check not base address for 0, but error code for 0 (NOERROR).



7.) not forget about 32-64 bit issues here (are your and target process have some bit-ness)



so code can look like:



ULONG GetModule(DWORD th32ProcessID, MODULEENTRY32* Entry, PCWSTR szModule)

HANDLE hSnapshot = CreateToolhelp32Snapshot(TH32CS_SNAPMODULE, th32ProcessID);

if (hSnapshot != INVALID_HANDLE_VALUE)

ULONG dwError = ERROR_NOT_FOUND;

Entry->dwSize = sizeof(MODULEENTRY32);

if (Module32FirstW(hSnapshot, Entry))

do

if (!_wcsicmp(Entry->szModule, szModule))

dwError = NOERROR;
break;

while (Module32NextW(hSnapshot, Entry));


CloseHandle(hSnapshot);

return dwError;


// return RtlGetLastNtStatus();
// this is much more better compare GetLastError();
return GetLastError();



and usage:



MODULEENTRY32 Entry;
if (ULONG dwError = GetModule(*, &Entry, L"*"))

// error, dont touch Entry

else

//ok, use Entry






share|improve this answer





















  • +1 You're answer makes some good points, but I disagree with #1. I believe many modern compilers can do copy elision and return value optimization, so passing a pointer or reference as an output is kind of old fashioned.
    – ncalmbeblpaicr0011
    11 hours ago










  • @ncalmbeblpaicr0011 - may be in some case compiler and can internal optimize this for avoid extra copy. but i think programmer must understand that internally anyway to function will be pass pointer to caller allocated structure. are compiler can direct use this pointer instead local copy in function (MODULEENTRY32 Entry) is question. anyway i personal think that this is very bad style. and good style - always return error code from function (as result we also need return value for error reserve).
    – RbMm
    10 hours ago













up vote
1
down vote










up vote
1
down vote









more simply api, way not exist.



1.) wrong return type - MODULEENTRY32. really impossible return structure, which not fit generic register size. when compiler view T fn(..) * it silently change this to void fn(T* t, ...) * and in your code Entry will be copied, what really not need. you must not try return MODULEENTRY32 but pass pointer to it as function argument.



2.) you lost error value. the GetModule can fail by different reasons - can not exist process with such id (STATUS_INVALID_CID), you can fail open target process (STATUS_ACCESS_DENIED), module can simply not exist in process. without returned error value - you can not know reason why function fail. really the best practice always return win32 error code (or NTSTATUS) from function.



3.) why you use module name as ansi string ? this is always bad and error design. use unicode name format. and you will be not need convert it (i already not say that if module name (this is rarely and never for system modules) use non english chars - it can not have correct ansi name at all(and depend from system locale - you encode it on self comp in one locale, on another comp - can be another locale. but ansi name depend from it).



4.) so signature of function must be ULONG GetModule(DWORD th32ProcessID, MODULEENTRY32* Entry, PCWSTR szModule)



5.) use wcscmp is wrong here. we need case insensitive compare. - _wcsicmp



6.) Return the module base 0x0 if we don't find any module - how i say - we need return appropriate error code in this case, instead set base address at 0 (we can left it undefined). client code must check not base address for 0, but error code for 0 (NOERROR).



7.) not forget about 32-64 bit issues here (are your and target process have some bit-ness)



so code can look like:



ULONG GetModule(DWORD th32ProcessID, MODULEENTRY32* Entry, PCWSTR szModule)

HANDLE hSnapshot = CreateToolhelp32Snapshot(TH32CS_SNAPMODULE, th32ProcessID);

if (hSnapshot != INVALID_HANDLE_VALUE)

ULONG dwError = ERROR_NOT_FOUND;

Entry->dwSize = sizeof(MODULEENTRY32);

if (Module32FirstW(hSnapshot, Entry))

do

if (!_wcsicmp(Entry->szModule, szModule))

dwError = NOERROR;
break;

while (Module32NextW(hSnapshot, Entry));


CloseHandle(hSnapshot);

return dwError;


// return RtlGetLastNtStatus();
// this is much more better compare GetLastError();
return GetLastError();



and usage:



MODULEENTRY32 Entry;
if (ULONG dwError = GetModule(*, &Entry, L"*"))

// error, dont touch Entry

else

//ok, use Entry






share|improve this answer













more simply api, way not exist.



1.) wrong return type - MODULEENTRY32. really impossible return structure, which not fit generic register size. when compiler view T fn(..) * it silently change this to void fn(T* t, ...) * and in your code Entry will be copied, what really not need. you must not try return MODULEENTRY32 but pass pointer to it as function argument.



2.) you lost error value. the GetModule can fail by different reasons - can not exist process with such id (STATUS_INVALID_CID), you can fail open target process (STATUS_ACCESS_DENIED), module can simply not exist in process. without returned error value - you can not know reason why function fail. really the best practice always return win32 error code (or NTSTATUS) from function.



3.) why you use module name as ansi string ? this is always bad and error design. use unicode name format. and you will be not need convert it (i already not say that if module name (this is rarely and never for system modules) use non english chars - it can not have correct ansi name at all(and depend from system locale - you encode it on self comp in one locale, on another comp - can be another locale. but ansi name depend from it).



4.) so signature of function must be ULONG GetModule(DWORD th32ProcessID, MODULEENTRY32* Entry, PCWSTR szModule)



5.) use wcscmp is wrong here. we need case insensitive compare. - _wcsicmp



6.) Return the module base 0x0 if we don't find any module - how i say - we need return appropriate error code in this case, instead set base address at 0 (we can left it undefined). client code must check not base address for 0, but error code for 0 (NOERROR).



7.) not forget about 32-64 bit issues here (are your and target process have some bit-ness)



so code can look like:



ULONG GetModule(DWORD th32ProcessID, MODULEENTRY32* Entry, PCWSTR szModule)

HANDLE hSnapshot = CreateToolhelp32Snapshot(TH32CS_SNAPMODULE, th32ProcessID);

if (hSnapshot != INVALID_HANDLE_VALUE)

ULONG dwError = ERROR_NOT_FOUND;

Entry->dwSize = sizeof(MODULEENTRY32);

if (Module32FirstW(hSnapshot, Entry))

do

if (!_wcsicmp(Entry->szModule, szModule))

dwError = NOERROR;
break;

while (Module32NextW(hSnapshot, Entry));


CloseHandle(hSnapshot);

return dwError;


// return RtlGetLastNtStatus();
// this is much more better compare GetLastError();
return GetLastError();



and usage:



MODULEENTRY32 Entry;
if (ULONG dwError = GetModule(*, &Entry, L"*"))

// error, dont touch Entry

else

//ok, use Entry







share|improve this answer













share|improve this answer



share|improve this answer











answered 13 hours ago









RbMm

1513




1513











  • +1 You're answer makes some good points, but I disagree with #1. I believe many modern compilers can do copy elision and return value optimization, so passing a pointer or reference as an output is kind of old fashioned.
    – ncalmbeblpaicr0011
    11 hours ago










  • @ncalmbeblpaicr0011 - may be in some case compiler and can internal optimize this for avoid extra copy. but i think programmer must understand that internally anyway to function will be pass pointer to caller allocated structure. are compiler can direct use this pointer instead local copy in function (MODULEENTRY32 Entry) is question. anyway i personal think that this is very bad style. and good style - always return error code from function (as result we also need return value for error reserve).
    – RbMm
    10 hours ago

















  • +1 You're answer makes some good points, but I disagree with #1. I believe many modern compilers can do copy elision and return value optimization, so passing a pointer or reference as an output is kind of old fashioned.
    – ncalmbeblpaicr0011
    11 hours ago










  • @ncalmbeblpaicr0011 - may be in some case compiler and can internal optimize this for avoid extra copy. but i think programmer must understand that internally anyway to function will be pass pointer to caller allocated structure. are compiler can direct use this pointer instead local copy in function (MODULEENTRY32 Entry) is question. anyway i personal think that this is very bad style. and good style - always return error code from function (as result we also need return value for error reserve).
    – RbMm
    10 hours ago
















+1 You're answer makes some good points, but I disagree with #1. I believe many modern compilers can do copy elision and return value optimization, so passing a pointer or reference as an output is kind of old fashioned.
– ncalmbeblpaicr0011
11 hours ago




+1 You're answer makes some good points, but I disagree with #1. I believe many modern compilers can do copy elision and return value optimization, so passing a pointer or reference as an output is kind of old fashioned.
– ncalmbeblpaicr0011
11 hours ago












@ncalmbeblpaicr0011 - may be in some case compiler and can internal optimize this for avoid extra copy. but i think programmer must understand that internally anyway to function will be pass pointer to caller allocated structure. are compiler can direct use this pointer instead local copy in function (MODULEENTRY32 Entry) is question. anyway i personal think that this is very bad style. and good style - always return error code from function (as result we also need return value for error reserve).
– RbMm
10 hours ago





@ncalmbeblpaicr0011 - may be in some case compiler and can internal optimize this for avoid extra copy. but i think programmer must understand that internally anyway to function will be pass pointer to caller allocated structure. are compiler can direct use this pointer instead local copy in function (MODULEENTRY32 Entry) is question. anyway i personal think that this is very bad style. and good style - always return error code from function (as result we also need return value for error reserve).
– RbMm
10 hours ago













 

draft saved


draft discarded


























 


draft saved


draft discarded














StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f200698%2fget-module-entry-by-name-from-external-program-via-winapi%23new-answer', 'question_page');

);

Post as a guest













































































Popular posts from this blog

Greedy Best First Search implementation in Rust

Function to Return a JSON Like Objects Using VBA Collections and Arrays

C++11 CLH Lock Implementation