Re: RSABASE/TESTS: Fix signed/unsigned comparison warnings
"Hans Leidekker" <hans(a)it.vu.nl> wrote:
+static const int size_of_wchar = sizeof(WCHAR); + ... - ok(!memcmp(bufW, stringW, 5 * sizeof(WCHAR)), "bufW/stringW mismatch\n"); + ok(!memcmp(bufW, stringW, 5 * size_of_wchar), "bufW/stringW mismatch\n");
- if (GetLastError()==NTE_BAD_KEYSET) + LONG gle = GetLastError(); + if (gle==NTE_BAD_KEYSET)
This is ugly. This kind of "fixes" clutters Wine code and should be deprecated. -- Dmitry.
On Monday 09 August 2004 11:17, Dmitry Timoshkov wrote:
- if (GetLastError()==NTE_BAD_KEYSET) + LONG gle = GetLastError(); + if (gle==NTE_BAD_KEYSET)
This is ugly. This kind of "fixes" clutters Wine code and should be deprecated.
What do you propose then? I could do something like this: if (GetLastError()==(DWORD)NTE_BAD_KEYSET) But I consider a cast even uglier then introducing an intermediate variable. We somehow have to live with the fact that NTE_BAD_KEYSET is defined as ((HRESULT)0x80090016L), which is signed, and GetLastError() returning DWORD, i.e unsigned. -Hans
On Monday 09 August 2004 12:17, Hans Leidekker wrote:
variable. We somehow have to live with the fact that NTE_BAD_KEYSET is defined as ((HRESULT)0x80090016L), which is signed, and GetLastError() returning DWORD, i.e unsigned.
On a related note, I am looking at the last testsuite with signed/unsigned comparison warnings, ntdll, and I'm trying to deal with constructs like this one: NTSTATUS ntstatus; ok(ntstatus == STATUS_SUCCESS, "Call failed (%lu)\n", ntstatus); where Wine defines STATUS_SUCCESS like so: #define STATUS_SUCCESS 0x00000000 which means STATUS_SUCCESS is handled by gcc as an unsigned value. This generates a warning because NTSTATUS is signed. If we look at the SDK definition we see something different: #define STATUS_SUCCESS ((NTSTATUS)0x00000000L) Which would get rid of the warnings for us if we add the cast there too. Should I submit a patch to do this? To add a cast in every such code sequence is not a good alternative if you ask me, as it's a very common idiom. Any other solutions? -Hans
Hans Leidekker wrote:
where Wine defines STATUS_SUCCESS like so:
#define STATUS_SUCCESS 0x00000000
which means STATUS_SUCCESS is handled by gcc as an unsigned value. This generates a warning because NTSTATUS is signed. If we look at the SDK definition we see something different:
#define STATUS_SUCCESS ((NTSTATUS)0x00000000L)
Which would get rid of the warnings for us if we add the cast there too. Should I submit a patch to do this? To add a cast in every such code sequence is not a good alternative if you ask me, as it's a very common idiom. Any other solutions?
Seems reasonable to fix Wine's headers to me. I think there's quite a few values like that to fix :/ Mike
"Hans Leidekker" <hans(a)it.vu.nl> wrote:
What do you propose then? I could do something like this:
if (GetLastError()==(DWORD)NTE_BAD_KEYSET)
But I consider a cast even uglier then introducing an intermediate variable. We somehow have to live with the fact that NTE_BAD_KEYSET is defined as ((HRESULT)0x80090016L), which is signed, and GetLastError() returning DWORD, i.e unsigned.
I propose a simple solution - just ignore warnings in these cases. Win32 is not an API which you could safely compile with signed/unsigned comparison warnings turned on, there are huge amount of cases when an API receives an unsigned int as a string length but returns signed int as a resulting length, and vice versa. There is no any consistency with passing/returning buffer lengths in Win32 at all, sometimes it's DWORD, sometimes UINT, sometimes just int. And that's just a couple of examples. -- Dmitry.
participants (3)
-
Dmitry Timoshkov -
Hans Leidekker -
Mike McCormack