Re: [PATCH spice-gtk 2/2] Don't include super-header from channel-main.h

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2015-09-22 at 11:40 -0400, Marc-André Lureau wrote:
> Hi
> 
> ----- Original Message -----
> > 
> > 
> > ----- Original Message -----
> > > On Tue, 2015-09-22 at 11:30 -0400, Marc-André Lureau wrote:
> > > > 
> > > > ----- Original Message -----
> > > > > Does anybody object to this series?
> > > > 
> > > > Yes, you broke the build with 1/2 and then you fix it in 2/2? and you
> > > > didn't explain what the point of it.
> > > 
> > > The point of it? channel-main.c should include channel-main.h. The fact
> > > that it was indirectly included was only an accident. I thought that
> > > would be self-explanatory. As for combining the two, that's a good
> > > point. Are you ok with it if I squash these?
> > 
> > Hmm, this could potentially break build of other users, since spice-client.h
> > is no longer included by channel-main.h.
> > 
> > Instead, I would suggest to add internal header guards, so only
> > spice-client.h can be included, and you get a compiler warning if not (see
> > how it's done with glib for ex)
> 
> spice-widget.h and documentation will need to be updated too.

spice-client.h only includes the non-gtk headers. So spice-widget.h is
not included there (nor is spice-gtk-session.h, etc). So I don't think
anything needs to change there. What did you have in mind?

Jonathon


> 
> > 
> > > Jonathon
> > > 
> > > 
> > > 
> > > > 
> > > > At the minimum, the two patches should be merged and explain why this
> > > > change is necessary.
> > > > 
> > > > > 
> > > > > On Mon, 2015-09-14 at 10:05 -0500, Jonathon Jongsma wrote:
> > > > > > This error only started occurring after patch 1/2.
> > > > > > 
> > > > > > 
> > > > > > On Fri, 2015-09-11 at 17:37 -0400, Marc-André Lureau wrote:
> > > > > > > Hi
> > > > > > > 
> > > > > > > ----- Original Message -----
> > > > > > > > spice-client.h is a super-header that includes all of the other
> > > > > > > > public
> > > > > > > > headers (including channel-main.h). By including this from
> > > > > > > > channel-main.h, it creates a circular dependency. This was
> > > > > > > > causing
> > > > > > > > some
> > > > > > > > build problems with spice-audio.h:
> > > > > > > > 
> > > > > > > > In file included from ../../src/spice-client.h:50:0,
> > > > > > > >                  from ../../src/channel-main.h:21,
> > > > > > > >                  from ../../src/channel-main.c:25:
> > > > > > > > ../../src/spice-audio.h:74:44: error: unknown type name
> > > > > > > > 'SpiceMainChannel'
> > > > > > > >                                             SpiceMainChannel
> > > > > > > >                                             *main_channel,
> > > > > > > >                                             ^
> > > > > > > > 
> > > > > > > 
> > > > > > > How did you get this error? after your patch or before?
> > > > > > > 
> > > > > > > > Instead of including this super-header, just include the one we
> > > > > > > > need:
> > > > > > > > spice-channel.h
> > > > > > > > ---
> > > > > > > >  src/channel-main.h | 2 +-
> > > > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > > > 
> > > > > > > > diff --git a/src/channel-main.h b/src/channel-main.h
> > > > > > > > index 86bb46b..92875dc 100644
> > > > > > > > --- a/src/channel-main.h
> > > > > > > > +++ b/src/channel-main.h
> > > > > > > > @@ -18,7 +18,7 @@
> > > > > > > >  #ifndef __SPICE_CLIENT_MAIN_CHANNEL_H__
> > > > > > > >  #define __SPICE_CLIENT_MAIN_CHANNEL_H__
> > > > > > > >  
> > > > > > > > -#include "spice-client.h"
> > > > > > > > +#include "spice-channel.h"
> > > > > > > >  
> > > > > > > >  G_BEGIN_DECLS
> > > > > > > >  
> > > > > > > > --
> > > > > > > > 2.1.0
> > > > > > > > 
> > > > > > > > _______________________________________________
> > > > > > > > Spice-devel mailing list
> > > > > > > > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > > > > > > > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> > > > > > > > 
> > > > > > 
> > > > > > 
> > > > > > _______________________________________________
> > > > > > Spice-devel mailing list
> > > > > > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > > > > > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> > > > > 
> > > > > 
> > > > > _______________________________________________
> > > > > Spice-devel mailing list
> > > > > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > > > > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> > > > > 
> > > 
> > > 
> > > _______________________________________________
> > > Spice-devel mailing list
> > > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> > > 
> > 


_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]