On Mon, 14 Aug 2023, Jiri Slaby wrote: > On 11. 08. 23, 12:26, Ilpo Järvinen wrote: > > On Thu, 10 Aug 2023, Jiri Slaby (SUSE) wrote: > > > > > Currently, the tty layer ops and functions use various types for same > > > things: > > > * characters and flags: unsigned char, char are used on a random basis, > > > * counts: int, unsigned int, size_t are used, again more-or-less > > > randomly. > > > > > > This makes it rather hard to remember where each type is required and it > > > also makes the code harder to follow. Also the code has to do min_t() on > > > many places simply because the variables hold the same kind of data, but > > > of different type. > > > > > > This is the first part of the series to unify the types: > > > * make characters and flags 'u8'. This is what the hardware expects and > > > what feeds the tty layer with. Since we compile with -funsigned-char, > > > char and unsigned char are the same types on all platforms. So there > > > is no actual change in type. > > > * make sizes/counts 'size_t'. This is what comes from the VFS layer and > > > some tty functions already operate on this. So instead of using > > > "shorter" (in term of bytes on 64bit) unsigned int, stick to size_t > > > and promote it to most places. > > > > > > More cleanup and spreading will be done in tty_buffer, n_tty, and > > > likely other places later. > > > > > > Patches 1-8 are cleanups only. The rest (the real switch) depends on > > > those. > > > > Yeah, very much needed change and step into the right direction! > > > > It's a bit tedious to review all this and comment a particular subchange > > but e.g. n_tty_receive_buf_common() still seems to still have int count > > which I think fall into the same call chain about size/count (probably > > most related change is #15). Note though that it also has room which I > > think can actually become negative so it might not be as straightforward > > search and replace like some other parts are. > > tl;dr > https://git.kernel.org/pub/scm/linux/kernel/git/jirislaby/linux.git/commit/?h=devel&id=9abb593df5a9b9b72d13438f1862ca67936f6b66 > > ---- > > Yes, sorry, my bad -- I forgot to elaborate on why this is "part I." and what > is going to be part II., III., ... > > So yeah, I have more in my queue which is growing a lot. I had to cut it at > some point as I was losing myself in all the changes already. So I flushed > this "part I.". It is only a minimalistic change in the core and necessary > changes in drivers' hooks. Parts II. and on will spread this more, of course. > Ideally, to every single loop in every driver ;) (in long-term). > > I still have a bunch of changes for tty_buffer and n_tty in my queue. As soon > as I rebase on the today's -next which is already supposed to contain this > part I., I will send part II. with these changes. I could have merged those > II. changes to some earlier I. patches. At first, I actually did try, but the > patches were growing with more and more dependencies, so I stopped this > approach. Instead, I separated the changes per the core/ldisc/drivers. The > parts are self-contained, despite it might look like the changes are > incomplete (i.e. not everything is changed everywhere). After all, I wanted to > avoid one hundred+ patches series. Yeah, right. Very much understandable. I realized you probably had more patches somewhere due to "Part I" designation but I couldn't check so I just noted the things that I came up during the review. -- i.
