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

[Peter Marschall]

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)

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


-- 
Peter Marschall
peter@adpm.de