From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Iy7Kl-0003Ii-ID for qemu-devel@nongnu.org; Fri, 30 Nov 2007 09:59:15 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Iy7Kj-0003IP-SM for qemu-devel@nongnu.org; Fri, 30 Nov 2007 09:59:14 -0500 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Iy7Kj-0003IM-Oc for qemu-devel@nongnu.org; Fri, 30 Nov 2007 09:59:13 -0500 Received: from outbound-sin.frontbridge.com ([207.46.51.80] helo=outbound10-sin-R.bigfish.com) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1Iy7Ki-0001kt-BE for qemu-devel@nongnu.org; Fri, 30 Nov 2007 09:59:13 -0500 Received: from outbound10-sin.bigfish.com (localhost.localdomain [127.0.0.1]) by outbound10-sin-R.bigfish.com (Postfix) with ESMTP id B34EF1852A4F for ; Fri, 30 Nov 2007 14:58:20 +0000 (UTC) Received: from mail176-sin-R.bigfish.com (unknown [10.3.40.3]) by outbound10-sin.bigfish.com (Postfix) with ESMTP id A9142198005B for ; Fri, 30 Nov 2007 14:58:20 +0000 (UTC) Received: from mail176-sin (localhost.localdomain [127.0.0.1]) by mail176-sin-R.bigfish.com (Postfix) with ESMTP id DBE901AB83B2 for ; Fri, 30 Nov 2007 14:57:53 +0000 (UTC) Received: from ausb3extmailp01.amd.com (unknown [163.181.251.8]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail176-sin.bigfish.com (Postfix) with ESMTP id 0C28F15680A4 for ; Fri, 30 Nov 2007 14:57:51 +0000 (UTC) Received: from SAUSGW02.amd.com (sausgw02.amd.com [163.181.250.22]) by ausb3extmailp01.amd.com (Switch-3.2.7/Switch-3.2.7) with ESMTP id lAUEwkYW022269 for ; Fri, 30 Nov 2007 08:59:02 -0600 Message-ID: <475024B7.3020906@amd.com> Date: Fri, 30 Nov 2007 15:56:55 +0100 From: "Andre Przywara" MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] fix gcc4 compile warnings References: <474F5223.1000202@andrep.de> <475019BF.4040906@amd.com> In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org andrzej zaborowski wrote: > On 30/11/2007, Andre Przywara wrote: >> > These casts are not the right way to get rid of the warnings, as are >> > some of the casts in other files in qemu_put_* and qemu_get_* >> > arguments. In this case the warnings are true positives and the bugs >> > causing the warnings have to be addressed instead of just the >> > warnings. >> >> Are you sure of that? Most of the fixes are like this: >> >> - qemu_put_be32s(f, &s->count_shift); >> >> + qemu_put_be32s(f, (uint32_t *)&s->count_shift); >> qemu_put_be32s is (QEMUFile *f, const uint32_t *pv), but after all the >> 2nd argument is only a _pointer_ to an unsigned variable, the size is >> the same (thanks to the C99 explicit types). > > count_shift is an int so it is the machine word size, not necesarily > 32-bit AFAIK. Otherwise I guess gcc wouldn't warn. You can possibly > use: > > qemu_put_be32(f, s->count_shift); > > Or point qemu_put_be32_s() to a int32_t/uint32_t variable. Oh, you are right, I missed that. Will rework the patch. >> What solution do you prefer for the opaque types? I have used the simple: >> >> - void *args[MAX_ARGS]; >> >> + intptr_t args[MAX_ARGS]; >> A more portable and clean solution would be this: >> - void *args[MAX_ARGS]; >> + union >> + { >> + void* ptr; >> + int i; >> + } args[MAX_ARGS]; >> If you prefer this, I can change the patch accordingly. > > I'm not sure why you get a warning here and I'm unable to run a build > at the moment. A void * should be able to store some (unknown size) > integer regardless of the platform. sizeof(void*) is 8, whereas sizeof(int) is 4 on a 64bit platform. If I assign a 32bit value to a 64bit (pointer) variable, GCC4 says: warning: cast to pointer from integer of different size You are right that a pointer _should_ be able to hold an integer, but AFAIK this is not guaranteed (aka written in the standard). To avoid those problems intptr_t was introduced. But I think opaque types in C are broken anyway, so the union version makes it at least more readable, since you explicitly say integer or pointer at the assignment and usage. But I have no problem with using intptr_t, the union was just a suggestion. What about fixing monitor.c#monitor_handle_command in general and avoid the opaque types at all? But wouldn't the union version work well even on win64? Regards, Andre. -- Andre Przywara AMD-OSRC (Dresden) Tel: x84917