[PATCH 04/11] xfree86: bus: remove superfluous and confused structures in BusRec

Vignatti Tiago (Nokia-D/Helsinki) tiago.vignatti at nokia.com
Fri May 21 09:50:56 PDT 2010


On Fri, May 21, 2010 at 04:40:59PM +0200, ext Keith Packard wrote:
> On Fri, 21 May 2010 14:43:17 +0300, Tiago Vignatti <tiago.vignatti at nokia.com> wrote:
> 
> > Although API is break, luckily any drivers right now is using such monster.
> 
> > diff --git a/hw/xfree86/common/xf86str.h b/hw/xfree86/common/xf86str.h
> > index 485c15a..e5713db 100644
> > --- a/hw/xfree86/common/xf86str.h
> > +++ b/hw/xfree86/common/xf86str.h
> > @@ -355,16 +355,10 @@ typedef enum {
> >  
> >  struct pci_device;
> >  
> > -typedef struct {
> > -    int		fbNum;
> > -} SbusBusId;
> > -
> >  typedef struct _bus {
> >      BusType type;
> > -    union {
> > -	struct pci_device *pci;
> > -	SbusBusId sbus;
> > -    } id;
> > +    struct pci_device *pci;
> > +    int sbus;
> >  } BusRec, *BusPtr;
> 
> This doeesn't make sense to me -- a bus is either an SBus or PCI, and so
> sticking those alternatives in a union (and nicely tagged at that with
> the 'type') is logical, even if it does add another level of naming to
> accessing the elements.

I'd say it does make sense but depends on the style of the programmer. But
c'mon, look again at this naming level:

-       return (pEnt->bus.id.sbus.fbNum == primaryBus.id.sbus.fbNum);
+       return (pEnt->bus.sbus == primaryBus.sbus);


the previous code seems pretty messy! 

Did I change your opinion? :)

        Tiago


More information about the xorg-devel mailing list