Re: Coding style, C++ compatible code (was Re: [Qemu-devel] [PATCH 02/22] eepro100: cast a void * makes no sense)

[Måns Rullgård]

Stefan Weil <address@hidden> writes:

> Markus Armbruster schrieb:
>> Stefan Weil <address@hidden> writes:
>>
>>> Juan Quintela schrieb:
>>>> Signed-off-by: Juan Quintela <address@hidden>
>>>> ---
>>>> 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 don't know the history of C++ either.  What I do know is that the
void* type was added to the C language precisely to *avoid* such
casts.  The original K&R C used char* as return type for malloc() etc,
and this of course required a cast.

> 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.

The declaration and assignment already make that perfectly clear.  The
cast is at best noise, and often hides a real error.

> 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.

Any syntax or semantics can potentially change in the future.  If *I*
were in charge, I'd make frivolous casts an *error*.

> 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).

That's your problem.  Please don't bring it here.

> 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).

Nor does it work in Fortran or COBOL.

> * 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.

In my personal projects, I never use typedefs for structs.  Typing
those 5 characters isn't much of a burden, and it makes it
unambiguously clear that something is a struct and not something
else.  It even works in C++, should you care.

C is not C++, even though much of the syntax is confusingly similar.
Trying to shoehorn the same coding style onto both is a mistake, and
does more harm than good whichever of the languages you are writing
in.

-- 
Måns Rullgård
address@hidden