Date:   Mon, 17 Jan 2022 19:13:57 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     "Maciej W. Rozycki" <macro@...ecosm.com>
Cc:     Jiri Slaby <jirislaby@...nel.org>, linux-serial@...r.kernel.org,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH] tty: Revert the removal of the Cyclades public API

On Mon, Jan 17, 2022 at 05:55:02PM +0000, Maciej W. Rozycki wrote:
> On Sat, 15 Jan 2022, Greg Kroah-Hartman wrote:
> 
> > On Fri, Jan 14, 2022 at 08:54:05PM +0000, Maciej W. Rozycki wrote:
> > > Fix a user API regression introduced with commit f76edd8f7ce0 ("tty: 
> > > cyclades, remove this orphan"), which removed a part of the API and 
> > > caused compilation errors for user programs using said part, such as 
> > > GCC 9 in its libsanitizer component[1]:
> > > 
> > > .../libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc:160:10: fatal error: linux/cyclades.h: No such file or directory
> > >   160 | #include <linux/cyclades.h>
> > >       |          ^~~~~~~~~~~~~~~~~~
> > > compilation terminated.
> > > make[4]: *** [Makefile:664: sanitizer_platform_limits_posix.lo] Error 1
> > 
> > So all we need is an empty header file?  Why bring back all of the
> > unused structures?
> 
>  Because they have become a part of the published API.  Someone may even 
> use a system using headers from the most recent version of the Linux 
> kernel (though not necessarily running such a kernel) to build software 
> intended to run on an older version that still does implement the API.  
> Times where people individually built pefectly matching software from 
> sources to run on each system they looked after have largely long gone.

For hardware-specific things like this, it's not the same.  I can see
adding the .h file as empty just to keep things building, but if no one
is actually ever using the structures and definitions in the file, it
should stay removed.

In looking at the file itself, it just looks like it wants a single
structure, struct cyclades_monitor, and then never actually does
anything with it.

So I guess I should submit a patch to the llvm developers to remove
these lines and add back the single structure definition to allow this
to keep building now?

> > > Any part of the public API is a contract between the kernel and the 
> > > userland and therefore once there it must not be removed even if its 
> > > implementation side has gone and any relevant calls will now fail 
> > > unconditionally.
> > 
> > Does this code actually use any of these structures?
> 
>  Well, they have been exported, so they have become a part of the API.  
> This user program may not use them, another one will.  If you don't want 
> an API to become public, then do not export it in the first place.

That happened a very long time ago, for hardware that no one has, sorry.

So the "ABI" broke when the driver was removed.  Given that no one has
the hardware, no one noticed the breakage, so there is no breakage :)

> > > Revert the part of the commit referred then that affects the user API, 
> > > bringing the most recent version of <linux/cyclades.h> back verbatim 
> > > modulo the removal of trailing whitespace which used to be there, and 
> > > updating <linux/major.h> accordingly.
> > 
> > Why major.h?  What uses that?  No userspace code should care about that.
> 
>  So it shouldn't have been a part of the user API in the first place.  
> Given that it has become a part of it it has to stay, that's the whole 
> point of having a user API.

But what user program uses that value?  I can't seem to find any, so
pointers would be appreciated.

> > Also, your text here is full of trailing whitespace, so I couldn't take
> > this commit as-is anyway :(
> 
>  Well, `git' is supposed to sort it automatically.  I've been routinely 
> feeding my patches as posted to `git am' for other projects so as to push 
> them and any trailing whitespace (added automatically by my e-mail client, 
> I guess for presentation purposes; not to be confused with `format=flowed' 
> arrangement as indicated by `Content-Type:', which I know to avoid) does 
> get stripped as the command executes, clearly prepared for this situation.
> 
>  The same must have happened for my earlier Linux kernel submissions ever 
> since the switch to `git' back in 2005 as they have been correctly applied 
> and no maintainer including you had an issue with it before.  And I have 
> been using the same e-mail client doing the same all over these years.
> 
>  To double-check I have just fed my submission as it to `git am' and it 
> did strip all the unwanted trailing whitespace.  Does that not happen with 
> your setup now?  Odd.

I did not check, I only responded to your email and saw the whitespace
in it.

I'll gladly take a patch that just adds the one needed structure to keep
this file building.  But that's all that is needed unless someone can
point out other code that needs these definitions.

thanks,

greg k-h

Powered by blists - more mailing lists