Re: [2.6.20.17 review 27/58] Include serial_reg.h with userspace headers

Willy Tarreau
2007-08-22T12:36:54Z

Hi Russell,

On Wed, Aug 22, 2007 at 10:23:32AM +0100, Russell King wrote:
> On Wed, Aug 22, 2007 at 11:39:11AM +0200, Willy Tarreau wrote:
> > As reported by Gustavo de Nardin <gustavodn@mandriva.com.br>, while trying to
> > compile xosview (http://xosview.sourceforge.net/) with upstream kernel
> > headers being used you get the following errors:
> > serialmeter.cc:48:30: error: linux/serial_reg.h: No such file or directory
> > serialmeter.cc: In member function 'virtual void
> > SerialMeter::checkResources()':
> > serialmeter.cc:71: error: 'UART_LSR' was not declared in this scope
> > serialmeter.cc:71: error: 'UART_MSR' was not declared in this scope
> > ..
>
> It's probably a good thing to repeat what I said privately about this,
> which is:
>
> | It should be noted that reading the MSR or LSR of an active 8250 UART
> | has side effects. In the case of MSR, it clears the bits which indicate
> | that the modem status signals have changed, and clears the associated
> | interrupt. This can result in loss of hardware flow control on the port.
(...)
> | For a tool which presumably is to assist diagnosing serial problems, it
> | would seem to have the capability of causing further additional problems.
>
> I leave it up to others to decide that keeping backwards compatibility
> with userspace which is quite obviously broken is more important than
> providing userspace with the persuasion to fix their broken code.

While I agree with you that side effects may be undesirable, I still think
it's important to ensure backwards compatibility in -stable branch, for the
following reasons :

- people who use -stable want to ensure that they can upgrade in order to
fix security issues or other bugs without risking (much) regression

- you do not prevent buggy programs from doing wrong things by removing
headers. What you will end up with is xosview using the quite common
#ifndef UART_LSR / #define UART_LSR / #endif. In fact, there *may* be
some legitimate uses of those defines, eventhough this program abuses
its capabilities.

Eventhough it's not a good practise to rely on kernel headers for userspace,
at least we should limit build regressions on the user side, as it does not
help the end user understand that the program he builds is wrong.

Thanks,
Willy