[PATCH] configure.ac: Remove unreachable check for VM86 headers.

Gaetan Nadon memsize at videotron.ca
Fri Sep 16 04:53:47 PDT 2011


On Thu, 2011-09-15 at 22:22 -0500, Jamey Sharp wrote:

> On Thu, Sep 15, 2011 at 07:28:25PM -0400, Gaetan Nadon wrote:
> > On Wed, 2011-09-14 at 10:07 -0500, Jamey Sharp wrote:
> > > "configure --with-int10=yes" is not a valid configuration, and the check
> > 
> > It depends what is the definition of "valid" in this context. Running
> > "./configure --with-int10" will set "INT10" to "yes". You may choose to
> > ignore this value. I can only guess that current code checked for "yes"
> > as a means to provide a default value when no backend was specified.
> 
> On further investigation: Daniel Stone introduced the block I'm
> proposing to delete in commit 588105173840355717d7b2f7f652289a41166c3f
> from 2005, with a commit message beginning "Huge cleanup." It appears
> that he intended that patch to mostly be a reorganization of
> configure.ac, not to change functionality, so I'm not sure how this
> snuck in.
> 
> Otherwise, as far as I can tell, there's never been an attempt to
> support INT10=yes.

That was my guess as well.

> 
> > > for sys/vm86.h and sys/io.h is not used. Delete it.
> > 
> > I agree with ignoring "yes". I don't recall any other module using it in
> > that way. Just be prepared in the case where someone did use it. It's
> > not really dead code, but close enough.
> > 
> > The default value is either x86emu or stub for FreeBSD on a PowerPC. Any
> > unrecognized value (such as yes, no or vn86) will not build any int10
> > backend. No warnings or errors. Hopefully the builder will have some way
> > of finding out why it does not work. The library builds with just the
> > common code.
> 
> "vn86" is an excellent example. This does seem error-prone. But:
> 
> > Suggestion:
> > 
> >         AS_HELP_STRING([--with-int10=BACKEND], [vm86, x86emu or stub (default:auto)]),
> >         
> >         AC_MSG_ERROR if no valid value is given
> 
> Can you suggest a patch that does that? I don't see any way to report
> such an error without copying the list of possible backends yet another
> time.

The vast majority of configure option do not make "user input
validation". If you think this option does not represent an above
average danger for the user, I am fine. Keep in mind I don't know the
server functionality :-)

> 
> >         Verify if there is a need to check for the headers. The code as
> >         it is was probably the result of changes around it rather than
> >         the intention of the developer.
> 
> Not as far as I can tell; and since nobody could have gotten a usable
> int10 module with --with-int10, and the AC_CHECK_HEADERS is only used in
> that case, I don't think anybody is relying on it.
> 
> So is there some reason not to apply this patch as-is?

Nope, but it would be nice to improve the help text in AS_HELP_STRING...
No need to resubmit just for that text change.

Reviewed-by: Gaetan Nadon <memsize at videotron.ca>

> 
> Thanks for looking this over!
> Jamey


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.x.org/archives/xorg-devel/attachments/20110916/52e32996/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <http://lists.x.org/archives/xorg-devel/attachments/20110916/52e32996/attachment.pgp>


More information about the xorg-devel mailing list