Re: [PATCH 3/8] passt.h: Include netinet/if_ether.h before struct ctx declaration

[Stefano Brivio <sbrivio@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1793 bytes --]

On Fri, 7 Oct 2022 19:40:43 +1100
David Gibson <david(a)gibson.dropbear.id.au> wrote:

> On Fri, Oct 07, 2022 at 09:44:17AM +0200, Stefano Brivio wrote:
> > On Fri, 7 Oct 2022 17:23:30 +1100
> > David Gibson <david(a)gibson.dropbear.id.au> wrote:
> >   
> > > On Fri, Oct 07, 2022 at 02:47:37AM +0200, Stefano Brivio wrote:  
> > > > This saves some hassle when including passt.h, as we need ETH_ALEN
> > > > there.
> > > > 
> > > > Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>    
> > > 
> > > Hrm.  So I had the impression that passt was using the convention that
> > > it's the top level files responsibility to include all the
> > > dependencies for a header before including the header, rather than
> > > having headers include other headers they need.  Was I mistaken?  I'm
> > > ok with either model, they each have their advantages, but I find
> > > sticking to one or the other is generally better than a mix of both.  
> > 
> > That was my original idea, but it's a bit of a disaster, because it
> > turns out we need <netinet/if_ether.h> and a few others pretty much
> > everywhere, even though the file at hand will never see an Ethernet
> > header. :(
> > 
> > Does this indicate that it's time to move struct ctx out of passt.h
> > (and similarly with other structs here and there)?  
> 
> Well.. really we want to break struct ctx up so it's not globals by
> another name, but that's not news.

Okay, that sounds reasonable, and at that point we can probably go back
to the original idea, which has a few advantages especially when
headers don't have include guards.

Until then I would say we could be flexible with it -- including
netinet/if_ether.h everywhere looks bad, but if we have an extra
include for stdint.h somewhere I don't see it as a drama.

-- 
Stefano