That's the definition yeah, but RegSetValueEx will (or at least, sometime in its' past did) happily write non-null terminated string data. Leading to:
A more thorough treatment of the problem can be found in the comment section hereOriginally Posted by http://msdn.microsoft.com/en-us/library/ms724911.aspx - RegQueryValueEx
I agree, it's something a solid application should check. However, if we assume that the user of the data is an application that is well-behaved, then I expect that the value stored would be correctly formed.
Checking for proper termination is not that hard:
Code:int checktermination(char *buffer, size_t bufsize, int nzeros) { size_t i; int j; for(i = bufsize - 1; i; i--) { if (!buffer[i]) { for(j = nzeros-1; j && i >= j; j--) { if (buffer[i-j]) break; } if (!j) { return 1; // Success. } } } return 0; }
Compilers can produce warnings - make the compiler programmers happy: Use them!
Please don't PM me for help - and no, I don't do help over instant messengers.
Incorrectly using RegSetValueEx will definitley cause issues. Strictly adhering to MS's guidelines on its use will definitely keep a programmer out of trouble.
I'd have to question the validity of the info on that posted link. Here's a link to a reputable resource. This resource states in section 8.2.3.2:
None of the many registry resources I've checked support the claims on the website.8.2.3.2. Getting and setting values
Let's start with something familiar: SHQueryValueEx is a dead ringer for RegQueryValueEx. It takes the same parameters and has the same restrictions and conditions, so it's pretty much a drop-in replacement.
Not sure that strlen() will work as expected in this case.
I think strlen() will return the number of chars until the FIRST terminator.
ie strlen("xxx/0yyy/0/0") == 3
The returned value lpcbData should be used as the size of the string, including terminators.
After the return from RegQueryValueEx has been checked for errors a terminator should be added at the lpcbData element of the string.
This is not tested code, trust it as much as any code written early Sunday morning......
Code:#define NUM_REG_STR 2//for ease I am assuming we have two values in the data //open reg key (hKey) //fill in subkey string (szSubKey) iRet=RegQueryValueEx (hKey, szSubKey, 0, &dwType, szRegData, &dwSize); if (iRet==ERROR_SUCCESS && dwSize && dwType==REG_MULTI_SZ)//got the right data { char *pTerm=NULL,*pStart=NULL;//pointers for string calcs char szReturn[NUM_REG_STR][MAX_PATH]={0}; int iRemaining=dwSize;//buffer remaining int iFound=0;//number of strings extracted //add terminator to end of string szRegData[dwSize]='\0'; while(iFound<NUM_REG_STR && iRemaining>0) { pStart=szRegData;//set the start of the string //find the first terminator pTerm=strchr(pStart,'\0');//find the first term if(pTerm)//found one { //print in the part of the string we need _snprintf(szReturn[iFound],pTerm-pStart,"%s",pStart); //increment our counters iFound++; iRemaining-=pTerm-pStart; //we only want to move past the terminators while we have more strings in the data if(iFound<NUM_REG_STR) { //move past the terminator while(*pTerm=='/0' && iRemaining>0) { pTerm++; iRemaining--; } //uodate the start pointer pStart=pTerm; } } //EDIT else break;//no more terminators, when we added one to the end??? } }
Last edited by novacain; 12-06-2008 at 08:24 PM. Reason: extra error check added
"Man alone suffers so excruciatingly in the world that he was compelled to invent laughter."
Friedrich Nietzsche
"I spent a lot of my money on booze, birds and fast cars......the rest I squandered."
George Best
"If you are going through hell....keep going."
Winston Churchill
Mea Culpa. Geoff's analysis is correct given the context (which is XP SP1 now that I've looked at it again), it was I who presumed the behaiour extended back to its creation. A quick ride on my time machine back to Win98 and NT4 and forwards to the present day reveals SHQueryValueEx guarantees correct NULL termination on any OS with any version of IE6 installed and by default from stock XP installs (shlwapi.dll version 6.0.2600.0). It's currently serving as nothing more than a wrapper of SHRegGetValue which is explicit about its string handling behaviour.
Edit: Ever have that feeling when you work something out and are confident of the result only for something to shatter that confidence moments later?
Empirical evidence suggests the preceeding is true for REG_SZ only. IE6 on 2k server SP4 won't properly terminate malformed REG_MULTI_SZ (XP SP3 does) which means that I'm an idiot and kicked up the fuss for nothing. Oh well, at least there was a potential buffer overflow so it hasn't been a complete waste.
Last edited by adeyblue; 12-07-2008 at 04:36 PM. Reason: We're not in Kansas anymore
Ok, I'm confused again. the website has posted this information concerning the early implementations of ShQueryValueEx
Note that it refers to Windows 98 SE and SHLWAPI build version 5.00. But Matt Pietrek in his article states that SHLWAPI did not come into existence until NT 5 which was alpha Windows 2000. All, the references I've checked and I've checked a lot of references, indicate that SHLWAPI requires Win 2000 SP1 or higher.In the case where pvData is not NULL but pcbData is NULL, versions before 5.00 fault because the function dereferences a NULL pointer when learning the buffer’s size. The first builds of version 5.00, for Internet Explorer 5.0 and Windows 98 SE, do not fix this correctly. When the function calls RegQueryValueEx, the local variable it uses for passing the buffer’s size and learning the amount of available data is uninitialised. A more or less random amount of memory at the address given by pvData may be corrupted.
Also, using the kernel debugger indicates that both the RegQueryValueEx and ShQueryValueEx map directly to the kernel NtQueryValue call. Which leads to another question. Why in the world would MS want to call another userland function to eventually get to a kernel function? I just dunno.
I believe Shlwapi didn't become part of the base / vanilla OS install until Win 2000, but it was available with IE.
Dll and Shell Version numbers
Since Wikipedia says that IE 4 was released in late 1997, that would put it's first release around the same time as the earliest 2k/5.0 betas. So it's plausible that shlwapi was in general use before Win2K went on general release.
Convenience? It's always expanded environment variables, so maybe someone got sick of making two calls and wrapped it up.Why in the world would MS want to call another userland function to eventually get to a kernel function? I just dunno.
Anyhoo, I don't know the guy so I couldn't honestly care less how straight his facts are now that I've been in and dug around myself. We've probably dragged this thread on long enough with pointless chatter in respect to the OP and actual thread topic, so I'm happy to continue via PM if you're so inclined.
Last edited by adeyblue; 12-07-2008 at 10:58 PM.
I find that the best way to deal with multi-strings (a null-terminated sequence of null-terminated strings) is to:
- replace all '\0' characters up to the length of the multistring with '\n' characters upon reading the string in
- perform any operations (such as strcat) that you so desire
- replace all '\n' characters in the string with '\0' characters upon writing the string back out
It's just sooo much easier to deal with, and if you think about it, that's what RegEdit does too!
My homepage
Advice: Take only as directed - If symptoms persist, please see your debugger
Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"
Here's some proof of concept code that anyone can play with to determine how RegQueryValueEx reacts to any type of data written to the key. The code first lists all the strings from the key and then it appends a new string to the key. So, you can modify the RegSetValueEx call to write any type of crazy data to the key whether or not that data is null terminated. Take the really incompetent programmer approach and get really creative (crazy) with what you write to the key.
You'll have to create the registry key prior to running the code.
Code:// The following registry key must exist prior to running app // HKEY_LOCAL_MACHINE\Software\TestKey\TestSubKey // TestSubKey is a REG_MULTI_SZ key #pragma comment ( lib, "advapi32.lib" ) #include <windows.h> #include <tchar.h> #include <stdio.h> #include <time.h> int main(void) { HKEY hKey = NULL; DWORD dwReturn = ERROR_SUCCESS; char szNewString[MAX_PATH] = {0}; srand(time(NULL)); _snprintf(szNewString,sizeof(szNewString), _T("NewString%d"),rand() % 100); dwReturn = RegOpenKeyEx( HKEY_LOCAL_MACHINE, TEXT("Software\\TestKey"), 0, KEY_ALL_ACCESS, &hKey ); if( dwReturn == ERROR_SUCCESS ) { DWORD dwSize; dwReturn = RegQueryValueEx( hKey, TEXT("TestSubKey"), 0, 0, 0, &dwSize ); if( dwReturn == ERROR_SUCCESS ) { DWORD dwType; DWORD dwNewSize = dwSize + _tcslen(szNewString) ; LPBYTE lpBuffer = LPBYTE(GlobalAlloc(GPTR, dwNewSize + 1)); if( lpBuffer == NULL ) { _tprintf("GlobalAlloc failed (%d)\n", GetLastError()); RegCloseKey(hKey); return -1; } dwReturn = RegQueryValueEx( hKey, TEXT("TestSubKey"), 0, &dwType, lpBuffer, &dwSize ); if( dwReturn == ERROR_SUCCESS ) { LPTSTR p = LPTSTR(lpBuffer); for(; *p; p += _tcslen(p) + 1) _tprintf("%s\n",p); if(!*p) { _tcscpy(p,szNewString ); dwReturn = RegSetValueEx( hKey, TEXT("TestSubKey"), 0, dwType, lpBuffer, dwNewSize ); if( dwReturn == ERROR_SUCCESS ) _tprintf("Registry update successful\n"); else _tprintf("Registry update FAILED\n"); } else _tprintf("pointer is not at end of buffer\n"); } else _tprintf("Second RegQueryValueEx failed\n"); GlobalFree(HGLOBAL(lpBuffer)); } else _tprintf("First RegQueryValueEx failed\n"); RegCloseKey(hKey); } else _tprintf("RegOpenKeyEx failed\n"); return 0; }
Here's something quite flexible,
I didn't test it after making it though...
Code:// var outam is optional - returns failure only when out of memory BOOL SplitMultiString(char *instr, char ***outstr, DWORD *outam) { DWORD a, i = 0, am = 1; for(a = 0; instr[a] || instr[a + 1]; a++) if(!instr[a]) am++; a = 0; char **out = new char*[am + 1]; if(!out) return FALSE; while(instr[a]) { DWORD len = lstrlen(&instr[a]); out[i] = new char[len + 1]; if(!out[i]) { for(a = 0; a < i; a++) delete [] out[a]; delete [] out; return FALSE; } lstrcpy(out[i++], &instr[a]); a += len + 1; } out[am] = NULL; *outstr = out; if(outam) *outam = am; return TRUE; } void FreeMultiString(char **outstr) { DWORD a; for(a = 0; outstr[a]; a++) delete [] outstr[a]; delete [] outstr; return; }
Last edited by Yarin; 12-09-2008 at 10:12 AM.
Hi all
Thanks for all your replies, RegSetValueEx requires that we have the multi string in the right format before writing it out, i.e. AAA\0BBB\0CCC\0\0 and I am having issues doing this.
This is what i am trying to do, basically I want to find if a data value exists in what i get from the registry (have done this already), if I find it I want to delete it, create the right format and then write back into the registry using the right format.
Below is what I have right now and I am stuck. I have read the REG_MULTI_SZ data from the registry and it is stored is params, I am basically checking if "XXX" is present in params[] and if I find it I am not writing it to tmp2 which is a pointer to an char array. As you can see strcat does not seem to help. Can someone help me with this Thanks!
Code:char* createMultiString(char *params[], int j, char *tmp2) { char *tmp=NULL; int i=0; int cpycount=0; int count=0; char value[MAX_PATH]={NULL}; sprintf (value,"XXX"); for (i=0; i<j;i++) { if (tmp2 == NULL) { printf ("In tmp2equal to NULL\n"); if (strcmp(params[i],value) != 0) { tmp2 = (char *)malloc(MAX_PATH); strcpy(tmp2,params[i]); } else { printf ("XXX data value found\n"); } } else { printf ("In tmp2notequaltoNULL\n"); if (strcmp(params[i],value) != 0) { cpycount = strlen(params[i]); int len2=0 char *ptr[] =params; char *ptr1; for(;;) { len2 =strlen[tmp2]; if (len2 == 0) { strncpy(tmp2,params[i],cpycount); } else { tmp2[count] = ptr1; count++; ptr1 += len2 + 1; //ptr += cpycount + 1; } } //strcat(tmp2,"\n"); //strcat(tmp2,params[i]); } else { printf ("XXX data value found\n"); } } } strcat(tmp2,"\n"); printf("---Final Value of Temp is %s after appending all Parameters\n",tmp2); return tmp2; }
You may want to consider possibly using unsigned char * as the type of input as opposed to char * to your function as a starting point. Keep in mind that the data returned by RegQueuryValueEx call has embedded NULL terminators. Also, as novacain previously pointed out, strlen etc. won't work with data returned by the call. This is also due to the embedded NULL terminators.
Actually I was able to use the code snippet in comment #2, to retrieve data from the registry without the null terminators and have written it to params.
>>You may want to consider possibly using unsigned char * as the type of input as opposed to >>char * to your function as a starting point
Can you let me know where you are pointing this too? Thanks.
Sometimes nobody listens to me. Seriously guys, this way of doing it only takes a handful of lines of code when you use bits of the standard library like replace. We use this technique successfully in production code and the only problem we've had is that someone once forgot to include a newline at the end of the string they were appending.
Nevermind using a long and untested piece of code for this. If I wasn't just about to head off to work I could give you a short and obviously correct piece of code to do it easily.
My homepage
Advice: Take only as directed - If symptoms persist, please see your debugger
Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"
iMalc,
I actually tried the approach that you suggested. But when I used RegSetValueEx to write to the registry, it writes just the first data value, not sure why. Thanks.
