This is the function I coded, obviously used to get the system name:
const char* os(){
OSVERSIONINFO os;
ZeroMemory(&os, sizeof(OSVERSIONINFO));
os.dwOSVersionInfoSize = sizeof(OSVERSIONINFO);
GetVersionEx(&os);
std::string ret = "Windows ";
if (os.dwMajorVersion == 10)
ret += "10";
else if (os.dwMajorVersion == 6){
if (os.dwMinorVersion == 3)
ret += "8.1";
else if (os.dwMinorVersion == 2)
ret += "8";
else if (os.dwMinorVersion == 1)
ret += "7";
else
ret += "Vista";
}
else if (os.dwMajorVersion == 5){
if (os.dwMinorVersion == 2)
ret += "XP SP2";
else if (os.dwMinorVersion == 1)
ret += "XP";
}
return ret.c_str();
}
This function returns a the pointer to a memory buffer which is deallocated at the function leaving (object dynamically allocated in the heap, not in the stack)
-
\$\begingroup\$ So, it's obviously broken, and you know it. Why are you posting here then? \$\endgroup\$Deduplicator– Deduplicator2015年10月18日 01:17:43 +00:00Commented Oct 18, 2015 at 1:17
-
\$\begingroup\$ If you know this results in a dangling pointer, why don't you just return the string? \$\endgroup\$Corbin– Corbin2015年10月18日 06:58:03 +00:00Commented Oct 18, 2015 at 6:58
1 Answer 1
Aggregate initialization
Prefer SecureZeroMemory
since according to MSDN, ZeroMemory
can be optimized out. Or better yet, don't use a funky macro:
OSVERSIONINFO os = {};
Invalid pointer
This function returns a the pointer to a memory buffer which is deallocated at the function leaving (object dynamically allocated in the heap, not in the stack)
That's a lot of mumbo-jumbo for "this function returns a pointer to garbage". Just return a std::string
and if the caller decides they want to do c_str()
, let them.
Version numbers
Some operating systems will have identical version numbers. For example, both Windows 8 and Windows Server 2012 are 6.2. The recommended way to distinguish them is to use OSVERSIONINFOEX.wProductType == VER_NT_WORKSTATION
(true for Windows 8, false for Windows Server 2012). A full table can be found here.
Better way?
Yes, the almighty documentation says:
To obtain the full version number for the operating system, call the GetFileVersionInfo function on one of the system DLLs, such as Kernel32.dll, then call VerQueryValue to obtain the \StringFileInfo\ProductVersion subblock of the file version information.
The order you must call the functions are GetFileVersionInfoSize
, GetFileVersionInfo
, then VerQueryValue
.
-
1\$\begingroup\$ Welcome to Code Review! Good job on your first answer. \$\endgroup\$SirPython– SirPython2015年10月18日 01:52:28 +00:00Commented Oct 18, 2015 at 1:52
-
\$\begingroup\$ Welcome to CR, and great answer! Small nitpick though: since the ZeroMemory call here is clearly affecting behavior, the compiler cannot legally optimize it out. SecureZeroMemory is typically used before you deallocate something that's consider sensitive (password, private key, etc). Since the memory is about to be released in that situation, the compiler can look at it and go "oh we're done with that anyway, so there's no reason to zero it out." In this case, that's not a concern since optimizing it out would break the program. \$\endgroup\$Corbin– Corbin2015年10月18日 06:56:55 +00:00Commented Oct 18, 2015 at 6:56