Re: [PATCH 4/5] xmllite/tests: Add basic test structure for IXmlReader
On 01/18/2010 10:28 PM, Nikolay Sivov wrote:
Add basic test structure for IXmlReader
Hi Nikolay, This one crashes on Vista without a servicepack at: + IXmlReader_Release(reader); +} All the calls before that one look ok (they produce the expected output). I guess "vista without an SP" can be considered broken(), but how do deal with that? -- Cheers, Paul.
On 1/21/2010 19:19, Paul Vriens wrote:
On 01/18/2010 10:28 PM, Nikolay Sivov wrote:
Add basic test structure for IXmlReader
Hi Nikolay,
This one crashes on Vista without a servicepack at:
+ IXmlReader_Release(reader); +}
All the calls before that one look ok (they produce the expected output).
I guess "vista without an SP" can be considered broken(), but how do deal with that?
That's a bit strange. Could you test for reader pointer around this: --- hr = pCreateXmlReader(&IID_IXmlReader, (LPVOID*)&reader, NULL); ok(hr == S_OK, "Expected S_OK, got %08x\n", hr); --- Add reader = 0xdeadbeef; and test what is reader after a call.
On 21 jan 2010, at 17:26, Nikolay Sivov <bunglehead(a)gmail.com> wrote:
On 1/21/2010 19:19, Paul Vriens wrote:
On 01/18/2010 10:28 PM, Nikolay Sivov wrote:
Add basic test structure for IXmlReader
Hi Nikolay,
This one crashes on Vista without a servicepack at:
+ IXmlReader_Release(reader); +}
All the calls before that one look ok (they produce the expected output).
I guess "vista without an SP" can be considered broken(), but how do deal with that?
That's a bit strange. Could you test for reader pointer around this: --- hr = pCreateXmlReader(&IID_IXmlReader, (LPVOID*)&reader, NULL); ok(hr == S_OK, "Expected S_OK, got %08x\n", hr); --- Add reader = 0xdeadbeef; and test what is reader after a call.
See https://winetestbot.geldorp.nl/JobDetails.pl?Key=386 Cheers, Paul
On 1/21/2010 19:49, Paul Vriens wrote:
On 21 jan 2010, at 17:26, Nikolay Sivov <bunglehead(a)gmail.com> wrote:
On 1/21/2010 19:19, Paul Vriens wrote:
On 01/18/2010 10:28 PM, Nikolay Sivov wrote:
Add basic test structure for IXmlReader
Hi Nikolay,
This one crashes on Vista without a servicepack at:
+ IXmlReader_Release(reader); +}
All the calls before that one look ok (they produce the expected output).
I guess "vista without an SP" can be considered broken(), but how do deal with that?
That's a bit strange. Could you test for reader pointer around this: --- hr = pCreateXmlReader(&IID_IXmlReader, (LPVOID*)&reader, NULL); ok(hr == S_OK, "Expected S_OK, got %08x\n", hr); --- Add reader = 0xdeadbeef; and test what is reader after a call.
See https://winetestbot.geldorp.nl/JobDetails.pl?Key=386 Hm. So it's created ok, at least looks so.
I don't see how to fix this. We could remove this _Release of course, but will leak.
Cheers,
Paul
2010/1/21 Nikolay Sivov <bunglehead(a)gmail.com>:
On 1/21/2010 19:49, Paul Vriens wrote:
On 21 jan 2010, at 17:26, Nikolay Sivov <bunglehead(a)gmail.com> wrote:
On 1/21/2010 19:19, Paul Vriens wrote:
On 01/18/2010 10:28 PM, Nikolay Sivov wrote:
Add basic test structure for IXmlReader
Hi Nikolay,
This one crashes on Vista without a servicepack at:
+ IXmlReader_Release(reader); +}
All the calls before that one look ok (they produce the expected output).
I guess "vista without an SP" can be considered broken(), but how do deal with that?
That's a bit strange. Could you test for reader pointer around this: --- hr = pCreateXmlReader(&IID_IXmlReader, (LPVOID*)&reader, NULL); ok(hr == S_OK, "Expected S_OK, got %08x\n", hr); --- Add reader = 0xdeadbeef; and test what is reader after a call.
Hm. So it's created ok, at least looks so.
I don't see how to fix this. We could remove this _Release of course, but will leak.
Can't you do something like: ULONG count; count = IXmlReader_AddRef(reader); ok (count == 2, "Expected AddRef to return 2, got: %d\n", count) while (count > 0) count = IXmlReader_Release(reader); ? - Reece
On 1/21/2010 20:41, Reece Dunn wrote:
2010/1/21 Nikolay Sivov<bunglehead(a)gmail.com>:
On 1/21/2010 19:49, Paul Vriens wrote:
On 21 jan 2010, at 17:26, Nikolay Sivov<bunglehead(a)gmail.com> wrote:
On 1/21/2010 19:19, Paul Vriens wrote:
On 01/18/2010 10:28 PM, Nikolay Sivov wrote:
Add basic test structure for IXmlReader
Hi Nikolay,
This one crashes on Vista without a servicepack at:
+ IXmlReader_Release(reader); +}
All the calls before that one look ok (they produce the expected output).
I guess "vista without an SP" can be considered broken(), but how do deal with that?
That's a bit strange. Could you test for reader pointer around this: --- hr = pCreateXmlReader(&IID_IXmlReader, (LPVOID*)&reader, NULL); ok(hr == S_OK, "Expected S_OK, got %08x\n", hr); --- Add reader = 0xdeadbeef; and test what is reader after a call.
Hm. So it's created ok, at least looks so.
I don't see how to fix this. We could remove this _Release of course, but will leak.
Can't you do something like:
ULONG count; count = IXmlReader_AddRef(reader); ok (count == 2, "Expected AddRef to return 2, got: %d\n", count) while (count> 0) count = IXmlReader_Release(reader);
Maybe. It's ugly of course, cause it's not clear at all why refcount could be zero just after creation with success code and not zero pointer. Btw, could you try it?
?
- Reece
2010/1/21 Nikolay Sivov <bunglehead(a)gmail.com>:
On 1/21/2010 20:41, Reece Dunn wrote:
2010/1/21 Nikolay Sivov<bunglehead(a)gmail.com>:
On 1/21/2010 19:49, Paul Vriens wrote:
On 21 jan 2010, at 17:26, Nikolay Sivov<bunglehead(a)gmail.com> wrote:
On 1/21/2010 19:19, Paul Vriens wrote:
On 01/18/2010 10:28 PM, Nikolay Sivov wrote:
> > Add basic test structure for IXmlReader > >
Hi Nikolay,
This one crashes on Vista without a servicepack at:
+ IXmlReader_Release(reader); +}
All the calls before that one look ok (they produce the expected output).
I guess "vista without an SP" can be considered broken(), but how do deal with that?
That's a bit strange. Could you test for reader pointer around this: --- hr = pCreateXmlReader(&IID_IXmlReader, (LPVOID*)&reader, NULL); ok(hr == S_OK, "Expected S_OK, got %08x\n", hr); --- Add reader = 0xdeadbeef; and test what is reader after a call.
Hm. So it's created ok, at least looks so.
I don't see how to fix this. We could remove this _Release of course, but will leak.
Can't you do something like:
ULONG count; count = IXmlReader_AddRef(reader); ok (count == 2, "Expected AddRef to return 2, got: %d\n", count) while (count> 0) count = IXmlReader_Release(reader);
Maybe. It's ugly of course, cause it's not clear at all why refcount could be zero just after creation with success code and not zero pointer. Btw, could you try it?
I don't have access to a Windows test machine to try it out, and don't have access to my home Linux dev box, so can't create a patch for the test bot at the moment. - Reece
On 01/21/2010 06:54 PM, Reece Dunn wrote:
2010/1/21 Nikolay Sivov<bunglehead(a)gmail.com>:
On 1/21/2010 20:41, Reece Dunn wrote:
2010/1/21 Nikolay Sivov<bunglehead(a)gmail.com>:
On 1/21/2010 19:49, Paul Vriens wrote:
On 21 jan 2010, at 17:26, Nikolay Sivov<bunglehead(a)gmail.com> wrote:
On 1/21/2010 19:19, Paul Vriens wrote:
> > On 01/18/2010 10:28 PM, Nikolay Sivov wrote: > >> >> Add basic test structure for IXmlReader >> >> > > Hi Nikolay, > > This one crashes on Vista without a servicepack at: > > + IXmlReader_Release(reader); > +} > > All the calls before that one look ok (they produce the expected > output). > > I guess "vista without an SP" can be considered broken(), but how do > deal with that? > >
That's a bit strange. Could you test for reader pointer around this: --- hr = pCreateXmlReader(&IID_IXmlReader, (LPVOID*)&reader, NULL); ok(hr == S_OK, "Expected S_OK, got %08x\n", hr); --- Add reader = 0xdeadbeef; and test what is reader after a call.
Hm. So it's created ok, at least looks so.
I don't see how to fix this. We could remove this _Release of course, but will leak.
Can't you do something like:
ULONG count; count = IXmlReader_AddRef(reader); ok (count == 2, "Expected AddRef to return 2, got: %d\n", count) while (count> 0) count = IXmlReader_Release(reader);
Maybe. It's ugly of course, cause it's not clear at all why refcount could be zero just after creation with success code and not zero pointer. Btw, could you try it?
I don't have access to a Windows test machine to try it out, and don't have access to my home Linux dev box, so can't create a patch for the test bot at the moment.
- Reece
Just checked and it doesn't help. The first release is fine but the second crashes again. It's really broken on Vista with no SP. -- Cheers, Paul.
2010/1/21 Paul Vriens <paul.vriens.wine(a)gmail.com>:
On 01/21/2010 06:54 PM, Reece Dunn wrote:
2010/1/21 Nikolay Sivov<bunglehead(a)gmail.com>:
On 1/21/2010 20:41, Reece Dunn wrote:
2010/1/21 Nikolay Sivov<bunglehead(a)gmail.com>:
On 1/21/2010 19:49, Paul Vriens wrote:
On 21 jan 2010, at 17:26, Nikolay Sivov<bunglehead(a)gmail.com> wrote:
> > On 1/21/2010 19:19, Paul Vriens wrote: > >> >> On 01/18/2010 10:28 PM, Nikolay Sivov wrote: >> >>> >>> Add basic test structure for IXmlReader >>> >>> >> >> Hi Nikolay, >> >> This one crashes on Vista without a servicepack at: >> >> + IXmlReader_Release(reader); >> +} >> >> All the calls before that one look ok (they produce the expected >> output). >> >> I guess "vista without an SP" can be considered broken(), but how do >> deal with that? >> >> > > That's a bit strange. Could you test for reader pointer around this: > --- > hr = pCreateXmlReader(&IID_IXmlReader, (LPVOID*)&reader, NULL); > ok(hr == S_OK, "Expected S_OK, got %08x\n", hr); > --- > Add reader = 0xdeadbeef; and test what is reader after a call. > > > >
Hm. So it's created ok, at least looks so.
I don't see how to fix this. We could remove this _Release of course, but will leak.
Can't you do something like:
ULONG count; count = IXmlReader_AddRef(reader); ok (count == 2, "Expected AddRef to return 2, got: %d\n", count) while (count> 0) count = IXmlReader_Release(reader);
Maybe. It's ugly of course, cause it's not clear at all why refcount could be zero just after creation with success code and not zero pointer. Btw, could you try it?
I don't have access to a Windows test machine to try it out, and don't have access to my home Linux dev box, so can't create a patch for the test bot at the moment.
- Reece
Just checked and it doesn't help. The first release is fine but the second crashes again.
It's really broken on Vista with no SP.
?_? Hmmm. According to http://msdn.microsoft.com/en-us/magazine/cc163436.aspx, this should work (but then msdn isn't always right). Do you know what happens if the CoGetMalloc/IMalloc_DidAlloc call is not made? According to that MSDN article, the CoInitialize is not needed. Does removing it make it work on Vista SP0? Shouldn't the IMalloc object be released? IMalloc_DidAlloc returns an int instead of a HRESULT (not a big issue, just can be confusing if someone expects HRESULT codes for the value of hr -- doing hr != 1 looks odd). Is the IXmlReader object valid (i.e. does a call to Read fail with an error that there is no input set, or does it crash)? XmlNodeType nodeType = XmlNodeType_None; hr = IXmlReader_Read(reader, &nodeType); ok(FAILED(hr), "IXmlReader_Read should fail!"); - Reece
On 01/21/2010 07:20 PM, Reece Dunn wrote:
?_?
Hmmm. According to http://msdn.microsoft.com/en-us/magazine/cc163436.aspx, this should work (but then msdn isn't always right).
Do you know what happens if the CoGetMalloc/IMalloc_DidAlloc call is not made?
If I leave this out the test doesn't crash.
According to that MSDN article, the CoInitialize is not needed. Does removing it make it work on Vista SP0?
Nope. But if that's the true the test should probably be changed to check for that?
Shouldn't the IMalloc object be released?
I thought it was just a pointer?
IMalloc_DidAlloc returns an int instead of a HRESULT (not a big issue, just can be confusing if someone expects HRESULT codes for the value of hr -- doing hr != 1 looks odd).
True, this could be written nicer. I leave that to Nikolay as he struggles through xmllite.
Is the IXmlReader object valid (i.e. does a call to Read fail with an error that there is no input set, or does it crash)?
XmlNodeType nodeType = XmlNodeType_None; hr = IXmlReader_Read(reader,&nodeType); ok(FAILED(hr), "IXmlReader_Read should fail!");
- Reece
Haven't checked that last one as we found that CoGetMalloc/IMalloc_DidAlloc is the culprit. It still only happens on Vista SP0 and looks like a valid call sequence, not? I'm not sure what the intention for this piece of code was btw. Was it to prove what allocated the memory (or rather what didn't)? -- Cheers, Paul.
On 1/22/2010 11:20, Paul Vriens wrote:
On 01/21/2010 07:20 PM, Reece Dunn wrote:
?_?
Hmmm. According to http://msdn.microsoft.com/en-us/magazine/cc163436.aspx, this should work (but then msdn isn't always right).
Do you know what happens if the CoGetMalloc/IMalloc_DidAlloc call is not made?
If I leave this out the test doesn't crash. Feel free to remove it.
Shouldn't the IMalloc object be released?
I thought it was just a pointer? There's nothing to release. It's a static global pointer. I'm not sure what the intention for this piece of code was btw. Was it to prove what allocated the memory (or rather what didn't)? I was to check that reader isn't allocated with default IMalloc - MSDN tells that passing NULL for imalloc parameter will use default IMalloc. For now we could remove this test out to stop crash cause xmllite is empty yet and more tests for this allocator are needed further.
participants (3)
-
Nikolay Sivov -
Paul Vriens -
Reece Dunn