[PATCH 0/1] MR8353: amstream/tests: Add a few return value checks (Coverity).
From: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/amstream/tests/amstream.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c index 7b0c8aec981..3dc0143d75b 100644 --- a/dlls/amstream/tests/amstream.c +++ b/dlls/amstream/tests/amstream.c @@ -8900,7 +8900,8 @@ static void test_ddrawstream_mem_allocator(void) hr = IDirectDrawMediaStream_SetFormat(ddraw_stream, &rgb555_format, NULL); ok(hr == S_OK, "Got hr %#lx.\n", hr); - IDirectDrawMediaStream_GetDirectDraw(ddraw_stream, &ddraw); + hr = IDirectDrawMediaStream_GetDirectDraw(ddraw_stream, &ddraw); + ok(hr == S_OK, "Got hr %#lx.\n", hr); surface_desc.ddpfPixelFormat = rgb555_format.ddpfPixelFormat; hr = IDirectDraw_CreateSurface(ddraw, &surface_desc, &surface, NULL); ok(hr == S_OK, "got hr %#lx\n", hr); @@ -9313,7 +9314,8 @@ static void test_ddrawstream_set_format_dynamic(void) hr = IDirectDrawMediaStream_SetFormat(ddraw_stream, &rgb8_format, NULL); ok(hr == S_OK, "Got hr %#lx.\n", hr); - IDirectDrawMediaStream_GetDirectDraw(ddraw_stream, &ddraw); + hr = IDirectDrawMediaStream_GetDirectDraw(ddraw_stream, &ddraw); + ok(hr == S_OK, "Got hr %#lx.\n", hr); palette_entries[0].peRed = 0x12; palette_entries[1].peBlue = 0x34; palette_entries[2].peGreen = 0x56; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8353
I won't reject this, but that was kind of an intentional choice, taking it as a given that GetDirectDraw() will succeed there, and avoiding an unnecessary line since it's not the point of the test. Is Coverity really warning about every unchecked HRESULT? That seems surprising; there's quite a lot in Wine. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8353#note_106897
This merge request was approved by Elizabeth Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8353
On Tue Jun 17 19:51:01 2025 +0000, Elizabeth Figura wrote:
I won't reject this, but that was kind of an intentional choice, taking it as a given that GetDirectDraw() will succeed there, and avoiding an unnecessary line since it's not the point of the test. Is Coverity really warning about every unchecked HRESULT? That seems surprising; there's quite a lot in Wine. Here it warns because there is 14 calls of the same function, but 2 of them are different. I think it makes sense, you either test for it everywhere or don't test it at all.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8353#note_106901
On Tue Jun 17 19:51:01 2025 +0000, Nikolay Sivov wrote:
Here it warns because there is 14 calls of the same function, but 2 of them are different. I think it makes sense, you either test for it everywhere or don't test it at all. I think it's excessive frankly. If you're trying to use some interface for example, you'd write QueryInterface() once, with a test that it's exposed, but once you've confirmed that in a formal test you might as well just write it without checking the return value from then on. That's common in quartz tests, and we might then write many tests that use that interface. This is a similar case.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8353#note_106903
participants (3)
-
Elizabeth Figura (＠zfigura) -
Nikolay Sivov
-
Nikolay Sivov (＠nsivov)