linux-kernel - Re: [PATCH 1/5] usb: gadget: f

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <56D866A1.5060303@felipetonello.com>
Date:	Thu, 3 Mar 2016 16:30:25 +0000
From:	Felipe Ferreri Tonello <eu@...ipetonello.com>
To:	Clemens Ladisch <clemens@...isch.de>, linux-usb@...r.kernel.org
Cc:	linux-kernel@...r.kernel.org, Felipe Balbi <balbi@...nel.org>,
	Michal Nazarewicz <mina86@...a86.com>
Subject: Re: [PATCH 1/5] usb: gadget: f_midi: refactor state machine

Hi Clemens,

On 03/03/16 11:38, Clemens Ladisch wrote:
> Felipe Ferreri Tonello wrote:
>> On 02/03/16 21:09, Clemens Ladisch wrote:
>>> Felipe F. Tonello wrote:
>>>> This refactor results in a cleaner state machine code
>>>
>>> It increases the number of states, and now juggles two state variables.
>>> I cannot agree to it being cleaner.
>>
>> Yes, it increases the number of states. That was done in order to
>> actually implement a proper finite state machine with one state at a
>> time and a transition state.
> 
> I know, "clean" is subjective.

Clean is subjective, yes. However, based on our common sense and
experience we can discern on what is clean and what is not. There is
also good literature about the subject that we can always consider.

> But in what way was the old state
> machine not "proper"?

Because it didn't reflect all the correct and possible MIDI states and
there was no transitional state.

> 
> And how is handling two states (port->state and next_state) cleaner?
> As far as I can tell, the requirement for a separate variable comes not
> from any inherent complexity of the state machine itself, but only
> because the transmit_packet function was inlined.

next_state is a transitional state, thus the temporal nature.

This patch doesn't change any functionality. But the important thing
here is that it improves the driver maintainability by making the state
machine cleaner (which is one of the most important pieces of code of
the driver). I call it clean because on each circumstance of each state
it's clear on what is about to happen to the USB request and to the
port's buffers.

I confess I would not spend the time on it just for puritanisms, but I
found myself a hard time while debugging it.

-- 
Felipe

Download attachment "0x92698E6A.asc" of type "application/pgp-keys" (7196 bytes)