[Spice-devel] typedefs and header dependencies

[Victor Toso]

On Tue, May 24, 2016 at 04:58:36AM -0400, Frediano Ziglio wrote:
> > 
> > Hi,
> > 
> > On Mon, 2016-05-23 at 16:19 -0500, Jonathon Jongsma wrote:
> > > On Mon, 2016-05-23 at 15:17 -0500, Jonathon Jongsma wrote:
> > > > On Sun, 2016-05-22 at 04:19 -0400, Frediano Ziglio wrote:
> > > > 
> > > > > >  /* FIXME: move back to display-channel.h (once structs are private)
> > > > > >  */
> > > > > >  typedef struct Drawable Drawable;
> > > > > > -typedef struct DisplayChannelClient DisplayChannelClient;
> > > > > >  
> > > > > 
> > > > > We definitively should consider using struct Xxxx instead of moving
> > > > > these typedef everytime in order to compile...
> > > > 
> > > > Or create a spice-types.h header that contains forward declarations for
> > > > all
> > > > structs and include that in every other file. The includes really are
> > > > quite
> > > > a
> > > > tangled mess and it's not an easy problem to unravel. I spent some time
> > > > trying,
> > > > but it's very difficult to do in a gradual way. I've resisted doing
> > > > something
> > > > like this in the past, but I'm starting to warm up to it...
> > > > 
> > > > Jonathon
> > > 
> > > 
> > > So, I chatted (very briefly) with Frediano about this on IRC after sending
> > > this
> > > email and I thought we should gauge opinions on options here.
> > > 
> > > This typedef stuff really does make refactoring more difficult. When we
> > > move
> > > stuff around, we have to keep adding temporary typedefs to make things
> > > compile.
> > > Or we have to remove these typedefs to avoid redefining the same struct
> > > multiple
> > > types. So there are several options:
> > > 
> > > 1. Use 'struct Foo' throughout the code instead of 'Foo' so that we don't
> > > have
> > > to deal with the typedef mess.
> > 
> > yes, +1
> 
> The issue here is having a lot of "struct Xxxx".
> 
> I would propose a half way to this:
> typedef only in the public relative header (for instance DisplayChannel in
> display-challel.h). This should avoid most typedefs in .c files. Also is
> supposed that most function dealing with that type are declared in that file
> too so this way you can reuse the typedef. Other headers will use 
> struct Xxxx; (declaration) and struct Xxxx when referenced.
> 
> +1.

+1 as well to use only typedefs on headers if that solves the issue.

>
> > > 
> > > 2. the approach listed above (a spice-types.h file that simply typedefs all
> > > structs). This is a bit of a maintenance nightmare and needs to be kept up
> > > to
> > > date and for a lot of types that are relatively self-contained it's not
> > > even
> > > necessary to put them in a global header like this...
> > 
> > I am afraid that would be easy to forget about updating the file
> 
> There are some issues of this approach:
> - scalability, if all types get into a single file every type addition
>   will cause all files to be recompiled;
> - prone to exceptions:
>   - public headers have to keep their types;
>   - why moving self contained (like glz or encoders) into this file?
> 
> > > 
> > > 3. continue with the fragile approach we now have
> > 
> > I think fragile is too strong word
> 
> Surely not!
> -1!
> 
> > > 
> > > 4. use C++ :D
> > 
> > I would wait with this
> 
> Yes, I think this is a weak argument, there are lot more useful feature
> we can use for other stuff.
> 
> > > 
> > > Anything else I'm not thinking of? I'd like to hear opinions from as many
> > > regular contributors as possible so we have a rough consensus, but I'd
> > > really
> > > like to do something about it.
> > 
> > Personally I would go for the first option, but it would be nice to avoid
> > some
> > mass rename...
> > 
> > Pavel
> > 
> 
> Jonathon, is not clear your vote :)
> 
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel