Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <8a0b6de4-3459-76fb-9117-287e71e315f1@kernel.org>
Date:   Mon, 14 Aug 2023 08:59:42 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.14.0
To:     =?UTF-8?Q?Ilpo_J=c3=a4rvinen?= <ilpo.jarvinen@linux.intel.com>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-serial <linux-serial@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>
References: <20230810091510.13006-1-jirislaby@kernel.org>
 <728dc060-d38c-b8d4-783-2322115cf4b4@linux.intel.com>
From:   Jiri Slaby <jirislaby@kernel.org>
Content-Language: cs-CZ, en-US
Subject: Re: [PATCH 00/36] tty: type unifications -- part I.
In-Reply-To: <728dc060-d38c-b8d4-783-2322115cf4b4@linux.intel.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

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.

thanks,
-- 
js
suse labs