[alsa-devel] [PATCH 06/39] ALSA: seq: obsolete address mode in compatibility layer

[Takashi Iwai]

On Tue, 09 Aug 2016 14:32:30 +0200,
Takashi Sakamoto wrote:
> 
> On Aug 9 2016 21:15, Takashi Iwai wrote:
> > But, why do you need to change these changes at all?  What you
> > basically need is to change the call inside seq_call_port_info_ioctl()
> > like:
> > 
> > -	err = snd_seq_do_ioctl(client, cmd, data);
> > +	err = snd_seq_kernel_client_ctl(client->number, cmd, data);
> > 
> > Then the rest can remain intact.  Referencing client->number in each
> > caller side just results in inefficient codes, obviously.
> 
> In seq_call_port_info_ioctl(), no members except for the 'number' are
> used. So no need to get a pointer to the client data as an argument.
> 
> This change makes it simpler for readers to follow code of the function.
> They have no need to think about the whole data, it's OK just to focus
> on the numerical ID for client.

I don't buy this argument, it's no obvious improvement about
readability at all, sorry.  Why delegating the object becomes so
difficult to follow suddenly from that point?  IOW, why passing a
client number there is easier to understand?  The caller doesn't need
to care which field is evaluated.  C isn't OOP language, but still
most of Linux codes are in such a style.

Unless any obvious code readability improvement is done, usually the
merit of the code optimization wins.  It even avoids the unnecessary
changes, makes easier to follow the change history in git tree in this
case.  It's often user-friendlier than small frequent changes here and
there "just for a taste".


Takashi