| Message ID | 51486eb6860d1680c1bce45e310dcd3aae096f43.1251111439.git.quintela@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
Juan Quintela schrieb: > Signed-off-by: Juan Quintela <quintela@redhat.com> > --- > 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. 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. So please don't apply this patch. Regards Stefan Weil
Stefan Weil <Stefan.Weil@weilnetz.de> wrote: > Juan Quintela schrieb: >> >> static void nic_reset(void *opaque) >> { >> - EEPRO100State *s = (EEPRO100State *) opaque; >> + EEPRO100State *s = opaque; >> logout("%p\n", s); >> static int first; >> if (!first) { Hi > I wrote these type casts, and I think they make sense. In C, they made no sense. they are a nop. In qemu, they are not used almost anywhere. > In C++ code, they are even mandatory. This is no C++. > I think the arguments why C++ requires this kind of > type casts apply to C code (like in Qemu) as well. And they are? I don't see what they buy us. > If it is possible with no or very litte efforts to write > code which is C and C++ compatible, I prefer to do so. If it is possible to have a consistent code base, with little effort, I prefer to do so. > So please don't apply this patch. I think it should be applied (qemu sense), but you do eepro100 work, and I don't do/plan to do more eepro100 work. I guess it is your call. Later, Juan.
Stefan Weil <Stefan.Weil@weilnetz.de> writes: > Juan Quintela schrieb: >> Signed-off-by: Juan Quintela <quintela@redhat.com> >> --- >> 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.
Juan Quintela wrote: > I think it should be applied (qemu sense), but you do eepro100 work, and > I don't do/plan to do more eepro100 work. > I agree with Juan. Overall consistency in the code base is extremely important. Stefan, if you disagree with this style, you should submit a cleanup patch that converts every occurrence of this idiom. Regards, Anthony Liguori
>> static void nic_save(QEMUFile * f, void *opaque) >> { >> - EEPRO100State *s = (EEPRO100State *) opaque; >> + EEPRO100State *s = opaque; > I wrote these type casts, and I think they make sense. Why? There is no technical reason for it. And in that case it IMHO doesn't make sense to keep the cast as documentation. > If it is possible with no or very litte efforts to write > code which is C and C++ compatible, I prefer to do so. Point being? I doubt qemu will go C++ ... cheers, Gerd
Markus Armbruster schrieb: > Stefan Weil <Stefan.Weil@weilnetz.de> writes: > >> Juan Quintela schrieb: >>> Signed-off-by: Juan Quintela <quintela@redhat.com> >>> --- >>> 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
Hi, > 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. There are quite a few cases where this will simply not work. They usually use a slightly different declaration style though: typedef struct T T; struct T { /* stuff here */ }; Reasons why this is used/needed: (1) structs pointing to each other, like this: typedef struct A A; typedef struct B B; struct A { B *b; }; struct B { A *a; }; (2) keep the struct private, let others pass around pointers to the struct in a typesafe way though: header file: typedef struct T T; T* get_foo(args); int do_something_with_foo(T*); source file: struct T { /* stuff here */ }; cheers, Gerd
Stefan Weil <weil@mail.berlios.de> writes: > Markus Armbruster schrieb: >> Stefan Weil <Stefan.Weil@weilnetz.de> writes: >> >>> Juan Quintela schrieb: >>>> Signed-off-by: Juan Quintela <quintela@redhat.com> >>>> --- >>>> 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? We can certainly discuss how to write better C, but appealing to the authority of the C++ rationale is an argument I don't buy. I prefer not to attempt to write C code that behaves identically when interpreted as C++ (extern "C" seems safer and easier). Does this answer your question? > 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. I don't see much value in that, sorry. > 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. I can't read the ISO C committee's mind, but I believe such an incompatible change is extremely unlikely, given the pains they've taken not to break existing C code. > 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). How about extern "C"? [...]
Markus Armbruster wrote: > Stefan Weil <weil@mail.berlios.de> writes: > > We can certainly discuss how to write better C, but appealing to the > authority of the C++ rationale is an argument I don't buy. > C++ compatibility is not a goal of QEMU AFAIK. We make extensive use of the fact that you can use struct Foo or Foo as a declaration via the TAILQ_ENTRY macro. With your proposal, that macro would no longer work. Regards, Anthony Liguori
Stefan Weil <weil@mail.berlios.de> writes: > Markus Armbruster schrieb: >> Stefan Weil <Stefan.Weil@weilnetz.de> writes: >> >>> Juan Quintela schrieb: >>>> Signed-off-by: Juan Quintela <quintela@redhat.com> >>>> --- >>>> 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.
On 08/26/2009 04:52 PM, Stefan Weil wrote: >>> 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). > You are not alone. > 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. > Yes. > 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). > It does work in C++. > * 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. > It's often needed, for example to have a struct T * in T.
On Wed, Aug 26, 2009 at 04:20:14PM +0100, Måns Rullgård wrote: > Stefan Weil <weil@mail.berlios.de> writes: > > 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. There are additional points, having all those casts makes people used to them and generally makes it much harder to spot those that are just wrong (not that C++ does not have this issue, since basically all conversions/uses of void* are wrong, with C they are unavoidable). Having casts for malloc results also makes it needlessly hard to change the type. Use var = malloc(count * sizeof(*var)) and you only need to change the declaration, add a cast and you also have to change all places where such allocations are done.
On 08/26/2009 06:58 PM, Reimar Döffinger wrote: > > There are additional points, having all those casts makes people used to > them and generally makes it much harder to spot those that are just > wrong (not that C++ does not have this issue, since basically all > conversions/uses of void* are wrong, with C they are unavoidable). > Having casts for malloc results also makes it needlessly hard to change > the type. Use > var = malloc(count * sizeof(*var)) > and you only need to change the declaration, add a cast and you also > have to change all places where such allocations are done. > Or better, NEW() and NEW_ARRAY().
Gerd Hoffmann wrote: > Hi, > > >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. > > There are quite a few cases where this will simply not work. They > usually use a slightly different declaration style though: ... > (1) structs pointing to each other, like this: > > typedef struct A A; > typedef struct B B; You can use "typedef struct _A A" to be C++ compatible, but it fails to be shorter so I wouldn't recommend it ;-) -- Jamie
Avi Kivity wrote: > On 08/26/2009 04:52 PM, Stefan Weil wrote: > >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. > > Yes. Not really. Adding the cast can hide bugs. If you omit the cast, a C compiler will at least warn, and maybe error, if the original pointer is not "void *", or is "const void *" and you are removing the constness. With the cast, the C compiler will silently let you compile buggy code. In C++, results vary. You are supposed to use a base class pointer, and if you need a cast, you are supposed to use "static_cast<>" anyway. In C++, it's an error without the cast, but it can also be an error with the cast: g++ -Wall test.cc -c -Werror=old-style-cast test.cc: In function ‘S* foo(void*)’: test.cc:5: error: use of old-style cast struct S *foo(void *ptr) { return (struct S *)ptr; } Of course there are dirty tricks to let you omit the "void *" pointer altogether. Let's not go there :-) -- Jamie
On Wed, 26 Aug 2009, Jamie Lokier wrote: > Gerd Hoffmann wrote: > > Hi, > > > > >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. > > > > There are quite a few cases where this will simply not work. They > > usually use a slightly different declaration style though: > .. > > (1) structs pointing to each other, like this: > > > > typedef struct A A; > > typedef struct B B; > > You can use "typedef struct _A A" to be C++ compatible, but it fails > to be shorter so I wouldn't recommend it ;-) This is neither C nor C++ compatible, in fact it breaks both.
malc wrote: > > > (1) structs pointing to each other, like this: > > > > > > typedef struct A A; > > > typedef struct B B; > > > > You can use "typedef struct _A A" to be C++ compatible, but it fails > > to be shorter so I wouldn't recommend it ;-) > > This is neither C nor C++ compatible, in fact it breaks both. You'll have to explain that statement. It compiles fine for me in C and C++: typedef struct _A A; typedef struct _B B; struct _A { struct _B *b; }; struct _B { struct _A *a; }; A *foo(B *arg); Using this works equally well: struct _A { B *b; }; struct _B { A *a; }; -- Jamie
On Wed, 26 Aug 2009, Jamie Lokier wrote: > malc wrote: > > > > (1) structs pointing to each other, like this: > > > > > > > > typedef struct A A; > > > > typedef struct B B; > > > > > > You can use "typedef struct _A A" to be C++ compatible, but it fails > > > to be shorter so I wouldn't recommend it ;-) > > > > This is neither C nor C++ compatible, in fact it breaks both. > > You'll have to explain that statement. ISO/IEC 9899:1999 7.1.3#1 and 17.4.3.1.2 of n2315 (Draft of c++0x) mandate that those names are reserved. [..snip..]
malc wrote: > On Wed, 26 Aug 2009, Jamie Lokier wrote: > > malc wrote: > > > > > (1) structs pointing to each other, like this: > > > > > > > > > > typedef struct A A; > > > > > typedef struct B B; > > > > > > > > You can use "typedef struct _A A" to be C++ compatible, but it fails > > > > to be shorter so I wouldn't recommend it ;-) > > > > > > This is neither C nor C++ compatible, in fact it breaks both. > > > > You'll have to explain that statement. > > ISO/IEC 9899:1999 7.1.3#1 > and 17.4.3.1.2 of n2315 (Draft of c++0x) > > mandate that those names are reserved. The _A is just an example; just substitute an identifier you're happy with. Use lower case, or put the underscore at the end, or something else. It's actively unhelpful to say "that is not C/C++ compatible" without explaining that it's for an tangential reason which is easily avoided. -- Jamie
On Wed, Aug 26, 2009 at 07:08:55PM +0300, Avi Kivity wrote:
> Or better, NEW() and NEW_ARRAY().
ISTR this being discussed before, but there was some disagreement
regarding whether it is preferable to have:
QEMU_NEW(ptr);
or:
ptr = QEMU_NEW(type);
If the type is incorrect, the latter form would still at least yield a
warning (and now therefore a build failure). It seems slightly more
readable to me, so that's the form that I would have preferred...
Is there any reason that this wouldn't be accepted, or should I start
submitting patches?
Cheers,
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)
Signed-off-by: Juan Quintela <quintela@redhat.com> --- hw/eepro100.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)