Code question

[TimDTR]

I'm trying to get in the habit of making clean/correct code, so i'm asking you guys to let me know if you see anything.. no matter how small it is.. I could change in this simple code. I'm posting this here because there seems to be quite a few people to actually know what they are talking about.

#include <iostream>
#include <windows.h>
#include <tlhelp32.h>

using namespace std;

int main()
{

	HANDLE hProcessSnapShot;
	PROCESSENTRY32 pProcessEntry;

	cout << "DEBUG: Attempting to create CreateToolhelp32Snapshot()" << endl;
	hProcessSnapShot = CreateToolhelp32Snapshot(TH32CS_SNAPALL, 0);
	
	if (hProcessSnapShot != INVALID_HANDLE_VALUE) {
		cout << "DEBUG: hProcessSnapShot = " << hProcessSnapShot << endl;
	} else {
		cout << "ERROR: CreateToolhelp32Snapshot() - " << GetLastError() << endl;
		return 1;
	}

	cout << "DEBUG: Filling value of pProcessEntry.dwSize" << endl;
	pProcessEntry.dwSize = sizeof(PROCESSENTRY32);
	cout << "DEBUG: pProcessEntry.dwSize = " << pProcessEntry.dwSize << endl;

	if (Process32First(hProcessSnapShot, &pProcessEntry))
	{
		cout << "DEBUG: Starting process main loop..." << endl;
	} else {
		cout << "ERROR: Process32First()" << GetLastError() << endl;
		return 1;
	}

	do
	{
		cout << "[" << pProcessEntry.szExeFile << "]" << endl;
	} while (Process32Next(hProcessSnapShot, &pProcessEntry));

	CloseHandle(hProcessSnapShot);

	return 0;
}

[Cypher]

Your program isn't Unicode safe. You need to use TCHARs in place of chars or wchar_ts, and wrap your strings in the TEXT macro. Also, you'll want to use a macro to wrap the STL and resolve to either wide or regular versions of each class depending on the compiler flag.

e.g.

#ifdef _UNICODE
#define tstring wstring
#define tfstream wfstream
#define tifstream wifstream
#define tofstream wofstream
#define tstringstream wstringstream
#define tcout wcout
#define tcin wcin
#define tistream wistream
#define tostream wostream
#else
#define tstring string
#define tfstream fstream
#define tifstream ifstream
#define tofstream ofstream
#define tstringstream stringstream
#define tcout cout
#define tcin cin
#define tistream istream
#define tostream ostream
#endif // _UNICODE

There's a bunch missing from there that you might want but you get the idea.

Furthermore, your code would potentially leak handles if used in a more complex environment. You should write classes for things like handles so you can use RAII.

And oh, defining variables at the top of a function is poor C++, you should define them as close to the point of initialisation as possible, and if possible, initialisation should happen at the same time as the object is defined.

Lastly, your naming scheme is stupid. 'pProcessEntry' implies that the variable is a pointer, which it isn't.

[TimDTR]

Thanks a lot for the feedback, exactly what I was looking for