Re: [PATCH] tty: Revert the removal of the Cyclades public API
From: Maciej W. Rozycki
Date: Mon Jan 17 2022 - 12:55:11 EST
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.
> > 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.
> > 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.
> 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.
Or did you get confused with the formatting issues the header itself used
to have that I did not address as preexisting code? If so, then I could
reformat the body of the change, however due to the original defects it
wouldn't be a trivial revert anymore (arguably in that case there ought to
be two changes in a series then, first a genuine revert, followed by the
style fix). Let me know if that is what you desire and I'll adapt v2
accordingly.
Thank you for your review.
Maciej