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 <mems...@videotron.ca> > > Thanks for looking this over! > Jamey
signature.asc
Description: This is a digitally signed message part
_______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel