Message ID | 1547196148-12250-1-git-send-email-thuth@redhat.com |
---|---|
State | New |
Headers | show |
Series | HACKING: Clarify the paragraph about typedefs | expand |
On 11/01/19 09:42, Thomas Huth wrote: > 2.3. Typedefs > -Typedefs are used to eliminate the redundant 'struct' keyword. > +Typedefs can be used to eliminate the redundant 'struct' keyword. This is > +especially helpful for common types that are used all over the place. Since > +certain C compilers choke on duplicated typedefs, you should avoid them and > +declare a typedef only in one header file. For common types, you can use > +"include/qemu/typedefs.h" for example. Note that it is also perfectly fine to > +use forward struct definitions without typedefs for references in headers > +to avoid the problem with duplicated typedefs. > I agree 100% with the wording after "Since". However, I think the first part should be made stronger, not weaker. Typedefs are use to eliminate the redundant 'struct' keyword, since type names have a different style than other identifiers ("CamelCase" versus "snake_case"). Each struct should have a CamelCase name and a corresponding typedef. Since certain C compilers choke on duplicated typedefs, you should avoid them and declare a typedef only in one header file. For common types, you can use "include/qemu/typedefs.h" for example. However, as a metter of convenience it is also perfectly fine to use forward struct definitions instead of typedefs in headers and function prototypes; this avoids problems with duplicated typedefs and reduces the need to include headers from other headers. And, I would move it to CODING_STYLE since we are at it. :) Paolo
On 1/11/19 11:38 AM, Paolo Bonzini wrote: > On 11/01/19 09:42, Thomas Huth wrote: >> 2.3. Typedefs >> -Typedefs are used to eliminate the redundant 'struct' keyword. >> +Typedefs can be used to eliminate the redundant 'struct' keyword. This is >> +especially helpful for common types that are used all over the place. Since >> +certain C compilers choke on duplicated typedefs, you should avoid them and >> +declare a typedef only in one header file. For common types, you can use >> +"include/qemu/typedefs.h" for example. Note that it is also perfectly fine to >> +use forward struct definitions without typedefs for references in headers >> +to avoid the problem with duplicated typedefs. >> > > I agree 100% with the wording after "Since". However, I think the first > part should be made stronger, not weaker. > > Typedefs are use to eliminate the redundant 'struct' keyword, since type > names have a different style than other identifiers ("CamelCase" versus > "snake_case"). Each struct should have a CamelCase name and a > corresponding typedef. > > Since certain C compilers choke on duplicated typedefs, you should avoid > them and declare a typedef only in one header file. For common types, > you can use "include/qemu/typedefs.h" for example. However, as a metter > of convenience it is also perfectly fine to use forward struct > definitions instead of typedefs in headers and function prototypes; this > avoids problems with duplicated typedefs and reduces the need to include > headers from other headers. I suppose this is difficult to check with checkpatch ? It's easy to cross the border as I have proven many times. > And, I would move it to CODING_STYLE since we are at it. :) yes. C.
On Fri, 11 Jan 2019 13:12:23 +0100 Cédric Le Goater <clg@kaod.org> wrote: > On 1/11/19 11:38 AM, Paolo Bonzini wrote: > > On 11/01/19 09:42, Thomas Huth wrote: > >> 2.3. Typedefs > >> -Typedefs are used to eliminate the redundant 'struct' keyword. > >> +Typedefs can be used to eliminate the redundant 'struct' keyword. This is > >> +especially helpful for common types that are used all over the place. Since > >> +certain C compilers choke on duplicated typedefs, you should avoid them and > >> +declare a typedef only in one header file. For common types, you can use > >> +"include/qemu/typedefs.h" for example. Note that it is also perfectly fine to > >> +use forward struct definitions without typedefs for references in headers > >> +to avoid the problem with duplicated typedefs. > >> > > > > I agree 100% with the wording after "Since". However, I think the first > > part should be made stronger, not weaker. > > > > Typedefs are use to eliminate the redundant 'struct' keyword, since type > > names have a different style than other identifiers ("CamelCase" versus > > "snake_case"). Each struct should have a CamelCase name and a > > corresponding typedef. > > > > Since certain C compilers choke on duplicated typedefs, you should avoid > > them and declare a typedef only in one header file. For common types, > > you can use "include/qemu/typedefs.h" for example. However, as a metter > > of convenience it is also perfectly fine to use forward struct > > definitions instead of typedefs in headers and function prototypes; this > > avoids problems with duplicated typedefs and reduces the need to include > > headers from other headers. > > I suppose this is difficult to check with checkpatch ? It's easy to > cross the border as I have proven many times. With Thomas's series merged, a simple build with clang will catch the duplicated typedefs. Not sure it is worth the pain to teach checkpatch. > > > And, I would move it to CODING_STYLE since we are at it. :) > > yes. > > C. >
On 2019-01-11 11:38, Paolo Bonzini wrote:
[...]
> And, I would move it to CODING_STYLE since we are at it. :)
I just got some feedback in IRC already, and seems like I am not alone,
so let's discuss it here on the mailing list, too: What's the exact
difference between CODING_STYLE and HACKING? Some of the paragraphs in
HACKING sound rather mandatory and coding-style related, too...
Should we maybe merge the two files into one (e.g. called "CODING")?
Thomas
On 16/01/19 11:58, Thomas Huth wrote: >> And, I would move it to CODING_STYLE since we are at it. :) > I just got some feedback in IRC already, and seems like I am not alone, > so let's discuss it here on the mailing list, too: What's the exact > difference between CODING_STYLE and HACKING? Some of the paragraphs in > HACKING sound rather mandatory and coding-style related, too... > Should we maybe merge the two files into one (e.g. called "CODING")? I think they should be merged, yes. It doesn't have to be done immediately though. Paolo
On 1/16/19 4:58 AM, Thomas Huth wrote: > On 2019-01-11 11:38, Paolo Bonzini wrote: > [...] >> And, I would move it to CODING_STYLE since we are at it. :) > > I just got some feedback in IRC already, and seems like I am not alone, > so let's discuss it here on the mailing list, too: What's the exact > difference between CODING_STYLE and HACKING? Some of the paragraphs in > HACKING sound rather mandatory and coding-style related, too... > Should we maybe merge the two files into one (e.g. called "CODING")? Merging the files sounds reasonable to me.
diff --git a/HACKING b/HACKING index 0fc3e0f..aa6fc3f 100644 --- a/HACKING +++ b/HACKING @@ -100,7 +100,13 @@ pointer, you're guaranteed that it is used to modify the storage it points to, or it is aliased to another pointer that is. 2.3. Typedefs -Typedefs are used to eliminate the redundant 'struct' keyword. +Typedefs can be used to eliminate the redundant 'struct' keyword. This is +especially helpful for common types that are used all over the place. Since +certain C compilers choke on duplicated typedefs, you should avoid them and +declare a typedef only in one header file. For common types, you can use +"include/qemu/typedefs.h" for example. Note that it is also perfectly fine to +use forward struct definitions without typedefs for references in headers +to avoid the problem with duplicated typedefs. 2.4. Reserved namespaces in C and POSIX Underscore capital, double underscore, and underscore 't' suffixes should be
The paragraph about typedefs is very sparse and caused some trouble already: Is this mandatory coding style or just a recommendation? ... since this is the HACKING file and not in CODING_STYLE. And various versions of GCC and Clang disallow duplicated typedefs in certain language modes, so the "enforced" typedeffing repeatedly caused compile errors in the past. Thus let's reword this paragraph a little bit, so that it is clear that typedefs are welcome, but not a mandatory coding style. Also add some information about our include/qemu/typedefs.h file here since most newcomers are not aware of this file yet. Signed-off-by: Thomas Huth <thuth@redhat.com> --- HACKING | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)