dead stores in advapi32

Rob Shearman robertshearman at gmail.com
Fri Nov 21 09:42:07 CST 2008


2008/11/20 ricardo filipe <ricardo_barbano at 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.

-- 
Rob Shearman



More information about the wine-devel mailing list