[PATCH v4 3/3] xfree86: remove board and vendor identifier strings from the configuration path

[Vignatti Tiago (Nokia-D/Helsinki)]

On Wed, Jun 30, 2010 at 01:37:10AM +0200, ext Keith Packard wrote:
> On Tue, 29 Jun 2010 16:08:03 +0300, Tiago Vignatti <tiago.vignatti at nokia.com> wrote:
> 
> > diff --git a/hw/xfree86/doc/man/xorg.conf.man.pre b/hw/xfree86/doc/man/xorg.conf.man.pre
> > index 6b3636f..3443e9b 100644
> > --- a/hw/xfree86/doc/man/xorg.conf.man.pre
> > +++ b/hw/xfree86/doc/man/xorg.conf.man.pre
> > @@ -1455,9 +1455,6 @@ The entries that may be used in
> >  .B Monitor
> >  sections are described below.
> >  .TP 7
> > -.BI "VendorName  \*q" vendor \*q
> > -This optional entry specifies the monitor's manufacturer.
> > -.TP 7
> >  .BI "ModelName  \*q" model \*q
> >  This optional entry specifies the monitor's model.
> >  .TP 7
> 
> You appear to be patching the monitor section instead of the device
> section here.

Arghh, that's the problem to code when we have hangover, without thinking much
(You know, World Cup is running, Brazil is doing well, summer time in Finland
- yes exists! - , etc).

Anyway, I gonna fix this and send another version of the patch.

 
> > diff --git a/hw/xfree86/parser/Device.c b/hw/xfree86/parser/Device.c
> > index d71abc6..2675686 100644
> > --- a/hw/xfree86/parser/Device.c
> > +++ b/hw/xfree86/parser/Device.c
> > @@ -123,14 +123,9 @@ xf86parseDeviceSection (void)
> >  			has_ident = TRUE;
> >  			break;
> >  		case VENDOR:
> > -			if (xf86getSubToken (&(ptr->dev_comment)) != STRING)
> > -				Error (QUOTE_MSG, "Vendor");
> > -			ptr->dev_vendor = val.str;
> > -			break;
> >  		case BOARD:
> > -			if (xf86getSubToken (&(ptr->dev_comment)) != STRING)
> > -				Error (QUOTE_MSG, "Board");
> > -			ptr->dev_board = val.str;
> > +			xf86parseError (OBSOLETE_MSG, xf86tokenString());
> > +			xf86getSubToken (&(ptr->dev_comment));
> >  			break;
> 
> Just FYI -- I think there is a memory leak here as you're parsing and
> then not freeing the string. Not a big deal as it's in the config file
> stuff.

I'll take a look on it. I also seeing other minor defects inside the parser
code (bad free and some deadcode), and I'll be probably take a look on those
as well.

 
> Question to the crowd -- any opinion on whether this change has become
> too large for inclusion in 1.9? It isn't adding any code, and only
> changes the ABI in removing some useless struct fields.

I'd say to push the first and second of this set to 1.9, given their
importance. This third one concerns more clean up, so I'll work a bit more,
squash to my tree and eventually we can push later when the merge window is
opened.


Thanks for reviewing! 

             Tiago