On 8/2/2010 16:07, David Hedberg wrote:
On Mon, Aug 2, 2010 at 10:50 AM, Nikolay Sivovnsivov@codeweavers.com wrote:
Ok, this test is present already cause Desktop list is an empty list. So please change this test for mkid.cb magic value to _ILIsEmpty() in BindToObject() and all other occurrences too (InitializeTreeView() for example).
Is something like the attached patch acceptable? I have only changed the checks where it would not introduce an extra check for NULL, and I have also not touched debughlp.c. Should I be more radical and also change places that only checks pidl->mkid.cb?
It's fine I think, for only pidl->mkid.cb I'd say no for now, cause I'm no sure about it. All this check thing needs to be revised and simplified probably.
Also, the check in _ILIsDesktop() seems a bit counter-intuitive to me (interpreting it as "pidl&& (pidl->mkid.cb ? FALSE : TRUE)" is closer to what you would expect as the current version also considers NULL pidl's to be the desktop (or empty)), but I assume that the current meaning is the intended one.
I'm not aware about assumptions in code that NULL pidl means desktop. For now your change doesn't change functionality, but only clarifies things. Again, occurrences of "NULL means desktop" should be found and tested if possible, but later (if you'll feel brave enough after your gsoc project end, for example).