Patchwork [03/24] qemu-common.h: comment about usage rules

login
register
mail settings
Submitter Eduardo Habkost
Date Nov. 9, 2012, 2:56 p.m.
Message ID <1352473012-20500-4-git-send-email-ehabkost@redhat.com>
Download mbox | patch
Permalink /patch/198101/
State New
Headers show

Comments

Eduardo Habkost - Nov. 9, 2012, 2:56 p.m.
Every time we make a tiny change on a header file, we often find
circular header dependency problems. To avoid this nightmare, we need to
stop including qemu-common.h on other headers, and we should gradually
move the declarations from the catchall qemu-common.h header to their
specific headers.

This simply adds a comment documenting the rules about qemu-common.h,
hoping that people will see it before including qemu-common.h from other
header files, and before adding more declarations to qemu-common.h.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 qemu-common.h | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
Andreas Färber - Nov. 12, 2012, 9:57 p.m.
Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> Every time we make a tiny change on a header file, we often find
> circular header dependency problems. To avoid this nightmare, we need to
> stop including qemu-common.h on other headers, and we should gradually

"from other headers" as below?

> move the declarations from the catchall qemu-common.h header to their
> specific headers.
> 
> This simply adds a comment documenting the rules about qemu-common.h,
> hoping that people will see it before including qemu-common.h from other
> header files, and before adding more declarations to qemu-common.h.

This reminds me that I had once posted a patch moving a declaration I
had once added for Cocoa to a new ui/ui.h... seems it never made it to
master, I'll go search, maybe we can smuggle that in now. ;)

> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  qemu-common.h | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/qemu-common.h b/qemu-common.h
> index ac9985c..ea43bfa 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -1,5 +1,14 @@
>  
> -/* Common header file that is included by all of qemu.  */
> +/* Common header file that is included by all of qemu.

"QEMU", while at it.

> + *
> + * This file is supposed to be included only by .c files. No header file should
> + * depend on qemu-common.h, as this would easily lead to circular header
> + * dependencies.
> + *
> + * If a header files uses a definition from qemu-common.h, that definition

"a header file"

> + * must be moved to a separate header file, and the header that uses it
> + * must include that header.
> + */
>  #ifndef QEMU_COMMON_H
>  #define QEMU_COMMON_H
>  

I'll fix this up myself to spare you a resend and me another full review.

Regards,
Andreas
Eduardo Habkost - Nov. 12, 2012, 10:04 p.m.
On Mon, Nov 12, 2012 at 10:57:42PM +0100, Andreas Färber wrote:
> Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> > Every time we make a tiny change on a header file, we often find
> > circular header dependency problems. To avoid this nightmare, we need to
> > stop including qemu-common.h on other headers, and we should gradually
> 
> "from other headers" as below?

Both forms sounds equivalent, to my non-native-speaker ears. :-)

But I guess "including from other headers" is better.

> 
> > move the declarations from the catchall qemu-common.h header to their
> > specific headers.
> > 
> > This simply adds a comment documenting the rules about qemu-common.h,
> > hoping that people will see it before including qemu-common.h from other
> > header files, and before adding more declarations to qemu-common.h.
> 
> This reminds me that I had once posted a patch moving a declaration I
> had once added for Cocoa to a new ui/ui.h... seems it never made it to
> master, I'll go search, maybe we can smuggle that in now. ;)
> 
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> >  qemu-common.h | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/qemu-common.h b/qemu-common.h
> > index ac9985c..ea43bfa 100644
> > --- a/qemu-common.h
> > +++ b/qemu-common.h
> > @@ -1,5 +1,14 @@
> >  
> > -/* Common header file that is included by all of qemu.  */
> > +/* Common header file that is included by all of qemu.
> 
> "QEMU", while at it.
> 
> > + *
> > + * This file is supposed to be included only by .c files. No header file should
> > + * depend on qemu-common.h, as this would easily lead to circular header
> > + * dependencies.
> > + *
> > + * If a header files uses a definition from qemu-common.h, that definition
> 
> "a header file"

Oops. Thanks.

> 
> > + * must be moved to a separate header file, and the header that uses it
> > + * must include that header.
> > + */
> >  #ifndef QEMU_COMMON_H
> >  #define QEMU_COMMON_H
> >  
> 
> I'll fix this up myself to spare you a resend and me another full review.

Thanks!

> 
> Regards,
> Andreas
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>

Patch

diff --git a/qemu-common.h b/qemu-common.h
index ac9985c..ea43bfa 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -1,5 +1,14 @@ 
 
-/* Common header file that is included by all of qemu.  */
+/* Common header file that is included by all of qemu.
+ *
+ * This file is supposed to be included only by .c files. No header file should
+ * depend on qemu-common.h, as this would easily lead to circular header
+ * dependencies.
+ *
+ * If a header files uses a definition from qemu-common.h, that definition
+ * must be moved to a separate header file, and the header that uses it
+ * must include that header.
+ */
 #ifndef QEMU_COMMON_H
 #define QEMU_COMMON_H