HACKING: Clarify the paragraph about typedefs

[Qemu-devel] [PATCH] HACKING: Clarify the paragraph about typedefs

Posted by Thomas Huth 5 years, 3 months ago

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

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

Re: [Qemu-devel] [PATCH] HACKING: Clarify the paragraph about typedefs

Posted by Paolo Bonzini 5 years, 3 months ago

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

Re: [Qemu-devel] [PATCH] HACKING: Clarify the paragraph about typedefs

Posted by Cédric Le Goater 5 years, 3 months ago

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.

Re: [Qemu-devel] [PATCH] HACKING: Clarify the paragraph about typedefs

Posted by Greg Kurz 5 years, 3 months ago

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

[Qemu-devel] HACKING vs. CODING_STYLE (was: Re: [PATCH] HACKING: Clarify the paragraph about typedefs)

Posted by Thomas Huth 5 years, 3 months ago

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

Re: [Qemu-devel] HACKING vs. CODING_STYLE (was: Re: [PATCH] HACKING: Clarify the paragraph about typedefs)

Posted by Paolo Bonzini 5 years, 3 months ago

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

Re: [Qemu-devel] HACKING vs. CODING_STYLE (was: Re: [PATCH] HACKING: Clarify the paragraph about typedefs)

Posted by Eric Blake 5 years, 3 months ago

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.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org