2008/11/20 ricardo filipe ricardo_barbano@hotmail.com:
hi rob.
Hi Ricardo,
i have some dead stores in advapi32 files that i would like to clarify. first, what do you want to do on lines 173 and 194 of cred.c?
Line 173 is: ret = RegQueryValueExW(hkey, wszUserNameValue, 0, &type, (LPVOID)credential->UserName, &count); if (ret == ERROR_FILE_NOT_FOUND) { credential->UserName = NULL; ret = ERROR_SUCCESS; <--- Here } else if (ret != ERROR_SUCCESS) return ret; else if (type != REG_SZ) return ERROR_REGISTRY_CORRUPT; else buffer += count;
You can just remove that redundant assignment and the then redundant braces.
Line 193 is: ret = read_credential_blob(hkey, key_data, credential->CredentialBlob, &count); if (ret == ERROR_FILE_NOT_FOUND) { credential->CredentialBlob = NULL; ret = ERROR_SUCCESS; <--- Here } else if (ret != ERROR_SUCCESS) return ret;
The same applies, although the distance from the use of ret again is quite large, which increases the risk of bugs being introduced in the future. However, since the pattern in this function is to check the return value immediately after use and to not keep it for later, I think the risk is small.
now, there are also 3 dead assignments in cred.c, lines 992, 199 and 1064. i didn't understand what you meant in your last email about my patch that removed dead code in ole32, so since this is dead code i think i can remove it, but i'm not sure anymore since you confused me.
These are the same types of issue as was highlighted in rpcrt4. The way I see it is that there is a pattern in this function where data is being copied, the buffer pointer being updated and the buffer length used being updated and I see no need to break that pattern for the last case. Yes, there is a redundant assignment, but it doesn't matter - it isn't a bug and may help prevent a bug in the future just in case in the code does need to be changed or if the code is copy and pasted.