New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
return values of xmlutil functions are not being consistent... #4899
Comments
There are 2 consumers of XmlGetNamedItem: XmlGetAttribute:
and XmlGetAttributeEx
|
Looks like it's always returned S_FALSE for the XmlGetAttribute (and the methods that consume it), however I do have concerns with the following calls (note the WixStBA line numbers are wrong, as I am doing this check on my branch with local mods to WixStdBA):
|
|
|
|
I am trying to turn off ShowFilesInUse, however it looks like the return values of the xmlutil functions aren't being consistent.
The XmlGetAttributeEx returns E_NOTFOUND, but XmlGetAttribute returns S_FALSE if the attribute isn't found (and as such so does XmlGetAttributeNumber).
From Jacob Hoover:
"Since no is the default value, it would make sense for it to not be written to the manifest.
https://msdn.microsoft.com/en-us/library/ms767592(v=vs.85).aspx
If that documentation is correct...
Return Values
S_OK
The value returned if successful.
S_FALSE
The value when returning Null.
E_INVALIDARG
The value returned if the namedItem parameter is Null.
That leads me to believe that XmlGetAttributeNumber is returning S_FALSE not E_NOTFOUND. In which case, it's turning on the files in use dialog because of the previous value being read in from the last option it found.
I sure hope I am reading the docs wrong, or this is a rather large and long standing bug.
"
and the solution:
"Steven,
I think it's safe to say this is a bug, where the return values of the xmlutil functions aren't consistent. The XmlGetAttributeEx returns E_NOTFOUND, but XmlGetAttribute returns S_FALSE if the attribute isn't found (and as such so does XmlGetAttributeNumber).
This bug has existed since WIX 3.8, but if you would log it, we can bring it up for discussion in tomorrow's meeting.
The short term fix, if you can build Wix, is to change E_NOTFOUND to S_FALSE for anything that calls XmlGetAttribute inside of WixStdBA.
if (E_NOTFOUND == hr) <--- This compare.
{
hr = S_OK;
}
else if (SUCCEEDED(hr))
{
...
}
"
i.e. short term solution for me:
if (E_NOTFOUND == hr) <--- This compare.
{
hr = S_OK;
}
else if (SUCCEEDED(hr))
{
...
}
To:
if (S_FALSE == hr) <--- This compare.
{
hr = S_OK;
}
else if (SUCCEEDED(hr))
{
...
}
ps actually the whole text of this bug was from Jacob :) (thank you sir)
The text was updated successfully, but these errors were encountered: