From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MgIvx-0004S1-AZ for qemu-devel@nongnu.org; Wed, 26 Aug 2009 09:53:05 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MgIvo-0004Qy-9t for qemu-devel@nongnu.org; Wed, 26 Aug 2009 09:53:01 -0400 Received: from [199.232.76.173] (port=40525 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MgIvn-0004Qp-7u for qemu-devel@nongnu.org; Wed, 26 Aug 2009 09:52:55 -0400 Received: from moutng.kundenserver.de ([212.227.17.9]:59217) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MgIvm-0001xS-HP for qemu-devel@nongnu.org; Wed, 26 Aug 2009 09:52:54 -0400 Message-ID: <4A953E20.8080806@mail.berlios.de> Date: Wed, 26 Aug 2009 15:52:32 +0200 From: Stefan Weil MIME-Version: 1.0 Subject: [Qemu-devel] Coding style, C++ compatible code (was Re: [Qemu-devel] [PATCH 02/22] eepro100: cast a void * makes no sense) References: <51486eb6860d1680c1bce45e310dcd3aae096f43.1251111439.git.quintela@redhat.com> <4A928DF0.9000106@weilnetz.de> <87tyzxnwvb.fsf@pike.pond.sub.org> In-Reply-To: <87tyzxnwvb.fsf@pike.pond.sub.org> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori , Markus Armbruster Cc: qemu-devel@nongnu.org Markus Armbruster schrieb: > Stefan Weil writes: > >> Juan Quintela schrieb: >>> Signed-off-by: Juan Quintela >>> --- >>> hw/eepro100.c | 6 +++--- >>> 1 files changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/hw/eepro100.c b/hw/eepro100.c >>> index 0031d36..09083c2 100644 >>> --- a/hw/eepro100.c >>> +++ b/hw/eepro100.c >>> @@ -507,7 +507,7 @@ static void nic_selective_reset(EEPRO100State * s) >>> >>> static void nic_reset(void *opaque) >>> { >>> - EEPRO100State *s = (EEPRO100State *) opaque; >>> + EEPRO100State *s = opaque; >>> logout("%p\n", s); >>> static int first; >>> if (!first) { >>> @@ -1544,7 +1544,7 @@ static ssize_t nic_receive(VLANClientState >>> *vc, const uint8_t * buf, size_t size >>> >>> static int nic_load(QEMUFile * f, void *opaque, int version_id) >>> { >>> - EEPRO100State *s = (EEPRO100State *) opaque; >>> + EEPRO100State *s = opaque; >>> int i; >>> int ret; >>> >>> @@ -1634,7 +1634,7 @@ static int nic_load(QEMUFile * f, void >>> *opaque, int version_id) >>> >>> static void nic_save(QEMUFile * f, void *opaque) >>> { >>> - EEPRO100State *s = (EEPRO100State *) opaque; >>> + EEPRO100State *s = opaque; >>> int i; >>> >>> if (s->pci_dev) >>> >> I wrote these type casts, and I think they make sense. >> In C++ code, they are even mandatory. > > Yes, but this isn't C++. > >> I think the arguments why C++ requires this kind of >> type casts apply to C code (like in Qemu) as well. >> >> If it is possible with no or very litte efforts to write >> code which is C and C++ compatible, I prefer to do so. > > I respectfully disagree. Casts from "void *" to "T *" are pure noise. > Getting into the habit of writing noise casts runs the risk of silencing > warnings that flag real type errors. Hello Do you only disagree with my first sentence or with both sentences? Currently, I seem to be alone with my opinion (at least in qemu-devel). Nevertheless, the designers of C++ thought that casts from void * to T * were something very important. I don't know the history of their decision. I personally think that deriving a data type T from some bytes in memory which can contain anything is an operation which is worth being documented by the programmer, and this is exactly what the cast does. My main reason why I try to write portable code is my personal experience with code reuse and the future development of compilers. It is quite possible that some day C compilers will require type casts like C++ compilers do today. And even today I want to be able to share C code from QEMU with program code written in C++ (which makes a large part of my business work). Anthony, there is already a mixture of coding styles in QEMU (also regarding type casts). This is not surprising for an open source project with many contributors. Do we really need additional regulations? I think the existing ones (those in CODING_STYLE) are very good, and they are sufficient. I'd appreciate it very much if all code in QEMU would respect this documented coding style. Today, it does not, and there was an agreement that we do not write commits which only change the coding style (at least for white space). I suggest to stick to this agreement for non white space coding style, too. Let me give one more C/C++ example. Today, many data structures are declared like this: typedef struct T { ... } T; There is nothing wrong with it, but it can be improved in several ways: * The declaration does not work with C++ (yes, I know that many programmers are not interested in C++ compatibility for QEMU). * The declaration allows variable declarations using struct T var; or just T var; (which is the QEMU style). I think a declaration which does not enforce the correct style is less good. * The declaration contains noise (bad argument, but many people like speaking of noise) :-) Why don't we declare structures like this: typedef struct { ... } T;? I suggest this to be the new coding style for structure declarations because it is shorter, C++ compatible and unambiguous. Kind regards Stefan