[Lcdproc] Re: [PATCH] include asm/termbits.h instead of termios.h in hd44780-serial.c

[paul_c]

On Tuesday 17 April 2007 20:02, Peter Marschall wrote:
> Hi,
>
> On Tuesday, 17. April 2007 19:24, Lars Stavholm wrote:
> > Peter Marschall wrote:
> > > On Tuesday, 17. April 2007 01:32, FoX wrote:
> > >> This version includes <asm/temios.h> instead of <termios.h>. It also
> > >> fixes the compilation with our toolchain and seems less intrusive.
> > >> Chose the best of the two.
> > >
> > > I concur with Markus' answer: unconditionally replacing standard
> > > headers without checking that the change does not negatively affect
> > > other distributions it not the way to go.
> >
> > This is exactly why the autotools (autoconf, automake,
> > libtool) were developed. All we have to do is use it.
> >
> > > AFAIK the files in /usr/include/asm/ should not be used directly by
> > > user prgorams.
> > >
> > > Please try to come up with something like
> > >
> > > #ifdef __GEEXBOX__
> > > # include ....
> > > #endif
> > >
> > > where __GEEXBOX__ can be determined at configuration time.
> > >
> > > Alternatively fix the toolchain to adhere to the standards
> > > for Linux and BSD ;-)
> >
> > Maybe something like...
> >
> > In configure.in, add:
> >
> > AC_CHECK_HEADERS(termios.h)
> >
> > And then add:
> >
> > #if HAVE_TERMIOS_H
> > #include <termios.h>
> > #endif
> >
> > ...in config.h.in.
> >
> > I've forgotten what the original problem was,
> > but the above is a typical setup with autotools.
>
> I am aware of that, but I no not think that this is the right fix.
>
> According to the OP the Baud rate constants for his toolchain
> where defined in <asm/termios.h>.
> He does not tell whether <termios.h> exists.
>
> Your proposed fix will not work if
> - the OP's toolchain has a <termios.h>
> - <asm/termios.h> does not have the baud rate definition
>   (as e.g. in my distribution, a stock Debian etch)
>

AC_COMPILE_IFELSE([AC_LANG_PROGRAM([
                    @%:@include <termios.h>
                    @%:@ifdef B230400
                    @%:@error B230400 is defined.
                    @%:@endif])],
    [AC_DEFINE([HAVE_GEEXBOX_FIX], [1], [Set to 1 if asm/termios.h is 
needed])],
    [AC_MSG_RESULT([termios.h is OK])])

Checks that B230400 is defined somewhere along the line in termios.h, else 
defines HAVE_GEEBOX_FIX. The OP can then conditionally include 
<asm/termios.h>

Still doesn't tell us if <termios.h> is present and usable....


> Currently the problem only exists for the one system/tooolchain
> while it works for a lot of different other systems/toolchains.
> I.e. the "default" works, while the special case GeexBox
> does not.
>
> Ifeel that your proposal reverses the logic: it makes the
> HAVE_TERMIOS_H case the exception and the other case the norm.
> This does not look right to me.
> (In my experience as a software engineer I learned  that in order
> to cover exception I should try to check for exactly these
> exceptions.)
>
> That's why I suggested to find something that specific is specific
> only to the GeexBox toolchain and then (and only then) replace
> <termios.h> by the GeexBox alternative.
>
> Regards
> Peter

Regards, Paul.