Message ID | 20190730210718.GU4313@habkost.net (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [RFC] HACKING: Document 'struct' keyword usage | expand |
On 7/30/19 4:07 PM, Eduardo Habkost wrote: > Sometimes we use the 'struct' keyword to help us reduce > dependencies between header files. Document that practice. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > I wonder if this is too terse? Should we give examples? An example might be nice, but I like the wording - it makes it obvious that the header uses 'struct' but the .c should use the typedef. > --- > HACKING | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/HACKING b/HACKING > index 0fc3e0fc04..112685bdaf 100644 > --- a/HACKING > +++ b/HACKING > @@ -101,6 +101,8 @@ it points to, or it is aliased to another pointer that is. > > 2.3. Typedefs > Typedefs are used to eliminate the redundant 'struct' keyword. > +However, the 'struct' keyword may be sometimes used in header > +files to avoid unnecessary dependencies between headers.
On 30/07/2019 23.07, Eduardo Habkost wrote: > Sometimes we use the 'struct' keyword to help us reduce > dependencies between header files. Document that practice. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > I wonder if this is too terse? Should we give examples? > --- > HACKING | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/HACKING b/HACKING > index 0fc3e0fc04..112685bdaf 100644 > --- a/HACKING > +++ b/HACKING > @@ -101,6 +101,8 @@ it points to, or it is aliased to another pointer that is. > > 2.3. Typedefs > Typedefs are used to eliminate the redundant 'struct' keyword. > +However, the 'struct' keyword may be sometimes used in header > +files to avoid unnecessary dependencies between headers. See also the discussion earlier this year: https://www.mail-archive.com/qemu-devel@nongnu.org/msg586180.html ... and we should merge HACKING and CODING_STYLE finally (that was on my private TODO list, but I never found the time to do it). Thomas
On Tue, Jul 30, 2019 at 11:07 PM Eduardo Habkost <ehabkost@redhat.com> wrote: > Sometimes we use the 'struct' keyword to help us reduce > dependencies between header files. Document that practice. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > I wonder if this is too terse? Should we give examples? > --- > I am inclined to have the judgement very similar to Eric's - it seems to me the change is fine as is, so: Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com> > HACKING | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/HACKING b/HACKING > index 0fc3e0fc04..112685bdaf 100644 > --- a/HACKING > +++ b/HACKING > @@ -101,6 +101,8 @@ it points to, or it is aliased to another pointer that > is. > > 2.3. Typedefs > Typedefs are used to eliminate the redundant 'struct' keyword. > +However, the 'struct' keyword may be sometimes used in header > +files to avoid unnecessary dependencies between headers. > > 2.4. Reserved namespaces in C and POSIX > Underscore capital, double underscore, and underscore 't' suffixes should > be > -- > 2.21.0 > >
On Wed, Jul 31, 2019 at 10:35:31AM +0200, Thomas Huth wrote: > On 30/07/2019 23.07, Eduardo Habkost wrote: > > Sometimes we use the 'struct' keyword to help us reduce > > dependencies between header files. Document that practice. > > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > --- > > I wonder if this is too terse? Should we give examples? > > --- > > HACKING | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/HACKING b/HACKING > > index 0fc3e0fc04..112685bdaf 100644 > > --- a/HACKING > > +++ b/HACKING > > @@ -101,6 +101,8 @@ it points to, or it is aliased to another pointer that is. > > > > 2.3. Typedefs > > Typedefs are used to eliminate the redundant 'struct' keyword. > > +However, the 'struct' keyword may be sometimes used in header > > +files to avoid unnecessary dependencies between headers. > > See also the discussion earlier this year: > > https://www.mail-archive.com/qemu-devel@nongnu.org/msg586180.html Nice, it adds even more information than this patch. For reference, this is the patch at the URL above: -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 don't agree with the first two sentences, and I agree with what Paolo said here: https://www.mail-archive.com/qemu-devel@nongnu.org/msg586214.html ("I agree 100% with the wording after 'Since'. However, I think the first part should be made stronger, not weaker.") Paolo sent the following proposal: | 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. It seems perfect to me. Paolo, do I have your signed-off-by to send that in a patch? > > ... and we should merge HACKING and CODING_STYLE finally (that was on my > private TODO list, but I never found the time to do it). Agreed, but I prefer to fix one problem at a time.
On 01/08/19 20:50, Eduardo Habkost wrote: > On Wed, Jul 31, 2019 at 10:35:31AM +0200, Thomas Huth wrote: >> On 30/07/2019 23.07, Eduardo Habkost wrote: >>> Sometimes we use the 'struct' keyword to help us reduce >>> dependencies between header files. Document that practice. >>> >>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> >>> --- >>> I wonder if this is too terse? Should we give examples? >>> --- >>> HACKING | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/HACKING b/HACKING >>> index 0fc3e0fc04..112685bdaf 100644 >>> --- a/HACKING >>> +++ b/HACKING >>> @@ -101,6 +101,8 @@ it points to, or it is aliased to another pointer that is. >>> >>> 2.3. Typedefs >>> Typedefs are used to eliminate the redundant 'struct' keyword. >>> +However, the 'struct' keyword may be sometimes used in header >>> +files to avoid unnecessary dependencies between headers. >> >> See also the discussion earlier this year: >> >> https://www.mail-archive.com/qemu-devel@nongnu.org/msg586180.html > > Nice, it adds even more information than this patch. > > For reference, this is the patch at the URL above: > > -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 don't agree with the first two sentences, and I agree with what Paolo said > here: > https://www.mail-archive.com/qemu-devel@nongnu.org/msg586214.html > > ("I agree 100% with the wording after 'Since'. However, I think the first > part should be made stronger, not weaker.") > > Paolo sent the following proposal: > > | 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. > > It seems perfect to me. > > Paolo, do I have your signed-off-by to send that in a patch? Sure. Paolo
On 01/08/2019 21.23, Paolo Bonzini wrote: > On 01/08/19 20:50, Eduardo Habkost wrote: >> On Wed, Jul 31, 2019 at 10:35:31AM +0200, Thomas Huth wrote: >>> On 30/07/2019 23.07, Eduardo Habkost wrote: >>>> Sometimes we use the 'struct' keyword to help us reduce >>>> dependencies between header files. Document that practice. >>>> >>>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> >>>> --- >>>> I wonder if this is too terse? Should we give examples? >>>> --- >>>> HACKING | 2 ++ >>>> 1 file changed, 2 insertions(+) >>>> >>>> diff --git a/HACKING b/HACKING >>>> index 0fc3e0fc04..112685bdaf 100644 >>>> --- a/HACKING >>>> +++ b/HACKING >>>> @@ -101,6 +101,8 @@ it points to, or it is aliased to another pointer that is. >>>> >>>> 2.3. Typedefs >>>> Typedefs are used to eliminate the redundant 'struct' keyword. >>>> +However, the 'struct' keyword may be sometimes used in header >>>> +files to avoid unnecessary dependencies between headers. >>> >>> See also the discussion earlier this year: >>> >>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg586180.html >> >> Nice, it adds even more information than this patch. >> >> For reference, this is the patch at the URL above: >> >> -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 don't agree with the first two sentences, and I agree with what Paolo said >> here: >> https://www.mail-archive.com/qemu-devel@nongnu.org/msg586214.html >> >> ("I agree 100% with the wording after 'Since'. However, I think the first >> part should be made stronger, not weaker.") >> >> Paolo sent the following proposal: >> >> | Typedefs are use to eliminate the redundant 'struct' keyword, since type s/use/used/ >> | 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 s/metter/matter/ >> | 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. With the typos fixed: Reviewed-by: Thomas Huth <thuth@redhat.com>
diff --git a/HACKING b/HACKING index 0fc3e0fc04..112685bdaf 100644 --- a/HACKING +++ b/HACKING @@ -101,6 +101,8 @@ it points to, or it is aliased to another pointer that is. 2.3. Typedefs Typedefs are used to eliminate the redundant 'struct' keyword. +However, the 'struct' keyword may be sometimes used in header +files to avoid unnecessary dependencies between headers. 2.4. Reserved namespaces in C and POSIX Underscore capital, double underscore, and underscore 't' suffixes should be
Sometimes we use the 'struct' keyword to help us reduce dependencies between header files. Document that practice. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- I wonder if this is too terse? Should we give examples? --- HACKING | 2 ++ 1 file changed, 2 insertions(+)