Re: [PATCH] tty: Revert the removal of the Cyclades public API

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux