All of lore.kernel.org
 help / color / mirror / Atom feed
From: SF Markus Elfring <elfring@users.sourceforge.net>
To: Takashi Iwai <tiwai@suse.de>, alsa-devel@alsa-project.org
Cc: Arvind Yadav <arvind.yadav.cs@gmail.com>,
	Takashi Sakamoto <o-takashi@sakamocchi.jp>,
	kernel-janitors@vger.kernel.org,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: ALSA: nm256: Fine-tuning for three function implementations
Date: Tue, 28 Nov 2017 09:19:48 +0100	[thread overview]
Message-ID: <2cbef557-5f89-c630-e108-14ef2ce6b41a@users.sourceforge.net> (raw)
In-Reply-To: <s5hbmjmluba.wl-tiwai@suse.de>

>>>> There is a general source code transformation pattern involved.
>>>> So I find that it is systematic.
>>>>
>>>> But I did not dare to develop a script variant for the semantic patch
>>>> language (Coccinelle software) which can handle all special use cases
>>>> as a few of them are already demonstrated in this tiny patch series.
>>>
>>> Then you're doing everything by hands,
>>
>> I am navigating through possible changes around the pattern
>> “Use common error handling code” mostly manually so far.
>>
>>
>>> and can be wrong
>>
>> Such a possibility remains as usual.
> 
> "As usual" doesn't suffice.

There can be additional means be used to reduce the probability
of undesired side effects.


> It must be "almost perfect" for such a code refactoring.

Can you get the impression that the shown transformation patterns were correctly
applied for the source file “sound/pci/nm256/nm256.c”?


> The damage by a overseen mistake is much higher than the merit by such a patch.

Are there any more software developers and code reviewers available
who would like to point another programming mistake out for this Linux module?


> If the patch is about fixing a bug, it's a different story.

How do “deviations” from the coding style and the evolution of object code size
fit to this view here?


> Or it's about a really trivial change (e.g. your sizeof() conversion
> patches), I can check and apply easily.

My update selection can contain also trivial adjustments.


> But for other changes with more lines, it makes little sense.

Do you need any more information to see and eventually accept the sense again?


> Again, the risk of breakage increases while the merit is negligible.

We disagree about corresponding benefits at the moment.
Would any other contributors comment the situation a bit more?


>>> -- that's the heart of the problem.
>>
>> There might be related opportunities for further improvements.
>> Do you trust adjustments from an evolving tool more than
>> my concrete contributions?
> 
> Yes, loudly.

I noticed that the development status of tools which you might find nice
at the moment can be also questionable.


> I stop at this point, as the rest is simply a repeat from the previous mail.

Are you using a continuous integration system?

Regards,
Markus
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

WARNING: multiple messages have this Message-ID (diff)
From: SF Markus Elfring <elfring@users.sourceforge.net>
To: Takashi Iwai <tiwai@suse.de>, alsa-devel@alsa-project.org
Cc: Arvind Yadav <arvind.yadav.cs@gmail.com>,
	Takashi Sakamoto <o-takashi@sakamocchi.jp>,
	kernel-janitors@vger.kernel.org,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: ALSA: nm256: Fine-tuning for three function implementations
Date: Tue, 28 Nov 2017 08:19:48 +0000	[thread overview]
Message-ID: <2cbef557-5f89-c630-e108-14ef2ce6b41a@users.sourceforge.net> (raw)
In-Reply-To: <s5hbmjmluba.wl-tiwai@suse.de>

>>>> There is a general source code transformation pattern involved.
>>>> So I find that it is systematic.
>>>>
>>>> But I did not dare to develop a script variant for the semantic patch
>>>> language (Coccinelle software) which can handle all special use cases
>>>> as a few of them are already demonstrated in this tiny patch series.
>>>
>>> Then you're doing everything by hands,
>>
>> I am navigating through possible changes around the pattern
>> “Use common error handling code” mostly manually so far.
>>
>>
>>> and can be wrong
>>
>> Such a possibility remains as usual.
> 
> "As usual" doesn't suffice.

There can be additional means be used to reduce the probability
of undesired side effects.


> It must be "almost perfect" for such a code refactoring.

Can you get the impression that the shown transformation patterns were correctly
applied for the source file “sound/pci/nm256/nm256.c”?


> The damage by a overseen mistake is much higher than the merit by such a patch.

Are there any more software developers and code reviewers available
who would like to point another programming mistake out for this Linux module?


> If the patch is about fixing a bug, it's a different story.

How do “deviations” from the coding style and the evolution of object code size
fit to this view here?


> Or it's about a really trivial change (e.g. your sizeof() conversion
> patches), I can check and apply easily.

My update selection can contain also trivial adjustments.


> But for other changes with more lines, it makes little sense.

Do you need any more information to see and eventually accept the sense again?


> Again, the risk of breakage increases while the merit is negligible.

We disagree about corresponding benefits at the moment.
Would any other contributors comment the situation a bit more?


>>> -- that's the heart of the problem.
>>
>> There might be related opportunities for further improvements.
>> Do you trust adjustments from an evolving tool more than
>> my concrete contributions?
> 
> Yes, loudly.

I noticed that the development status of tools which you might find nice
at the moment can be also questionable.


> I stop at this point, as the rest is simply a repeat from the previous mail.

Are you using a continuous integration system?

Regards,
Markus

WARNING: multiple messages have this Message-ID (diff)
From: SF Markus Elfring <elfring@users.sourceforge.net>
To: Takashi Iwai <tiwai@suse.de>, alsa-devel@alsa-project.org
Cc: Arvind Yadav <arvind.yadav.cs@gmail.com>,
	Jaroslav Kysela <perex@perex.cz>,
	Takashi Sakamoto <o-takashi@sakamocchi.jp>,
	kernel-janitors@vger.kernel.org,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: ALSA: nm256: Fine-tuning for three function implementations
Date: Tue, 28 Nov 2017 09:19:48 +0100	[thread overview]
Message-ID: <2cbef557-5f89-c630-e108-14ef2ce6b41a@users.sourceforge.net> (raw)
In-Reply-To: <s5hbmjmluba.wl-tiwai@suse.de>

>>>> There is a general source code transformation pattern involved.
>>>> So I find that it is systematic.
>>>>
>>>> But I did not dare to develop a script variant for the semantic patch
>>>> language (Coccinelle software) which can handle all special use cases
>>>> as a few of them are already demonstrated in this tiny patch series.
>>>
>>> Then you're doing everything by hands,
>>
>> I am navigating through possible changes around the pattern
>> “Use common error handling code” mostly manually so far.
>>
>>
>>> and can be wrong
>>
>> Such a possibility remains as usual.
> 
> "As usual" doesn't suffice.

There can be additional means be used to reduce the probability
of undesired side effects.


> It must be "almost perfect" for such a code refactoring.

Can you get the impression that the shown transformation patterns were correctly
applied for the source file “sound/pci/nm256/nm256.c”?


> The damage by a overseen mistake is much higher than the merit by such a patch.

Are there any more software developers and code reviewers available
who would like to point another programming mistake out for this Linux module?


> If the patch is about fixing a bug, it's a different story.

How do “deviations” from the coding style and the evolution of object code size
fit to this view here?


> Or it's about a really trivial change (e.g. your sizeof() conversion
> patches), I can check and apply easily.

My update selection can contain also trivial adjustments.


> But for other changes with more lines, it makes little sense.

Do you need any more information to see and eventually accept the sense again?


> Again, the risk of breakage increases while the merit is negligible.

We disagree about corresponding benefits at the moment.
Would any other contributors comment the situation a bit more?


>>> -- that's the heart of the problem.
>>
>> There might be related opportunities for further improvements.
>> Do you trust adjustments from an evolving tool more than
>> my concrete contributions?
> 
> Yes, loudly.

I noticed that the development status of tools which you might find nice
at the moment can be also questionable.


> I stop at this point, as the rest is simply a repeat from the previous mail.

Are you using a continuous integration system?

Regards,
Markus

  reply	other threads:[~2017-11-28  8:19 UTC|newest]

Thread overview: 121+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-16 17:05 [PATCH 0/2] ALSA: nm256: Fine-tuning for three function implementations SF Markus Elfring
2017-11-16 17:05 ` SF Markus Elfring
2017-11-16 17:05 ` SF Markus Elfring
2017-11-16 17:07 ` [PATCH 1/2] ALSA: nm256: Adjust five function calls together with a variable assignment SF Markus Elfring
2017-11-16 17:07   ` SF Markus Elfring
2017-11-16 17:07   ` SF Markus Elfring
2017-11-16 17:08 ` [PATCH 2/2] ALSA: nm256: Use common error handling code in snd_nm256_probe() SF Markus Elfring
2017-11-16 17:08   ` SF Markus Elfring
2017-11-16 17:08   ` SF Markus Elfring
2017-11-16 17:15 ` [PATCH 0/2] ALSA: nm256: Fine-tuning for three function implementations Takashi Iwai
2017-11-16 17:15   ` Takashi Iwai
2017-11-16 17:15   ` Takashi Iwai
2017-11-16 17:48   ` SF Markus Elfring
2017-11-16 17:48     ` SF Markus Elfring
2017-11-16 17:48     ` SF Markus Elfring
2017-11-16 18:54     ` Takashi Iwai
2017-11-16 18:54       ` Takashi Iwai
2017-11-16 19:30       ` SF Markus Elfring
2017-11-16 19:30         ` SF Markus Elfring
2017-11-28  7:46         ` Takashi Iwai
2017-11-28  7:46           ` Takashi Iwai
2017-11-28  8:19           ` SF Markus Elfring [this message]
2017-11-28  8:19             ` SF Markus Elfring
2017-11-28  8:19             ` SF Markus Elfring
2017-11-28  9:10             ` Takashi Iwai
2017-11-28  9:10               ` Takashi Iwai
2017-11-28  9:10               ` Takashi Iwai
2017-11-28  9:50               ` SF Markus Elfring
2017-11-28  9:50                 ` SF Markus Elfring
2017-11-28  9:50                 ` SF Markus Elfring
2017-11-28 11:37                 ` Takashi Iwai
2017-11-28 11:37                   ` Takashi Iwai
2017-11-28 12:33                   ` SF Markus Elfring
2017-11-28 12:33                     ` SF Markus Elfring
2017-11-28 12:33                     ` SF Markus Elfring
2017-11-28 12:46                     ` Takashi Iwai
2017-11-28 12:46                       ` Takashi Iwai
2017-11-28 13:00                       ` SF Markus Elfring
2017-11-28 13:00                         ` SF Markus Elfring
2017-11-28 13:00                         ` SF Markus Elfring
2017-11-28 13:06                         ` Takashi Iwai
2017-11-28 13:06                           ` Takashi Iwai
2017-11-28 13:17                           ` SF Markus Elfring
2017-11-28 13:17                             ` SF Markus Elfring
2017-11-28 13:17                             ` SF Markus Elfring
2017-11-28 13:38                             ` Takashi Iwai
2017-11-28 13:38                               ` Takashi Iwai
2017-11-28 14:19                               ` SF Markus Elfring
2017-11-28 14:19                                 ` SF Markus Elfring
2017-11-28 14:19                                 ` SF Markus Elfring
2017-11-28 14:27                                 ` Takashi Iwai
2017-11-28 14:27                                   ` Takashi Iwai
2017-11-28 14:33                                   ` SF Markus Elfring
2017-11-28 14:33                                     ` SF Markus Elfring
2017-11-28 14:33                                     ` SF Markus Elfring
2017-11-28 14:38                                     ` Takashi Iwai
2017-11-28 14:38                                       ` Takashi Iwai
2017-11-28 14:38                                       ` Takashi Iwai
2017-11-28 14:44                                       ` SF Markus Elfring
2017-11-28 14:44                                         ` SF Markus Elfring
2017-11-28 14:44                                         ` SF Markus Elfring
2017-11-28 14:53                                         ` Takashi Iwai
2017-11-28 14:53                                           ` Takashi Iwai
2017-11-28 14:53                                           ` Takashi Iwai
2017-11-28 15:01                                           ` SF Markus Elfring
2017-11-28 15:01                                             ` SF Markus Elfring
2017-11-28 15:01                                             ` SF Markus Elfring
2017-11-28 15:21                                             ` Takashi Iwai
2017-11-28 15:21                                               ` Takashi Iwai
2017-11-28 15:21                                               ` Takashi Iwai
2017-11-28 16:15                                               ` SF Markus Elfring
2017-11-28 16:15                                                 ` SF Markus Elfring
2017-11-28 16:15                                                 ` SF Markus Elfring
2017-11-28 16:27                                                 ` Takashi Iwai
2017-11-28 16:27                                                   ` Takashi Iwai
2017-11-28 16:27                                                   ` Takashi Iwai
2017-11-28 16:40                                                   ` SF Markus Elfring
2017-11-28 16:40                                                     ` SF Markus Elfring
2017-11-28 16:40                                                     ` SF Markus Elfring
2017-11-28 16:44                                                     ` Takashi Iwai
2017-11-28 16:44                                                       ` Takashi Iwai
2017-11-28 16:44                                                       ` Takashi Iwai
2017-11-28 17:15                                                       ` SF Markus Elfring
2017-11-28 17:15                                                         ` SF Markus Elfring
2017-11-28 17:15                                                         ` SF Markus Elfring
2017-11-28 18:35                                                         ` Takashi Iwai
2017-11-28 18:35                                                           ` Takashi Iwai
2017-11-28 19:08                                                           ` SF Markus Elfring
2017-11-28 19:08                                                             ` SF Markus Elfring
2017-11-28 19:08                                                             ` SF Markus Elfring
2017-11-28 19:43                                                             ` Takashi Iwai
2017-11-28 19:43                                                               ` Takashi Iwai
2017-11-28 19:43                                                               ` Takashi Iwai
2017-11-28 19:48                                                               ` SF Markus Elfring
2017-11-28 19:48                                                                 ` SF Markus Elfring
2017-11-28 19:48                                                                 ` SF Markus Elfring
2017-11-28 19:54                                                                 ` Takashi Iwai
2017-11-28 19:54                                                                   ` Takashi Iwai
2017-11-28 19:54                                                                   ` Takashi Iwai
2017-11-28 19:57                                                                   ` SF Markus Elfring
2017-11-28 19:57                                                                     ` SF Markus Elfring
2017-11-28 19:57                                                                     ` SF Markus Elfring
2017-11-28 20:00                                                                     ` Takashi Iwai
2017-11-28 20:00                                                                       ` Takashi Iwai
2017-11-28 20:00                                                                       ` Takashi Iwai
2017-11-28 20:18                                                                       ` SF Markus Elfring
2017-11-28 20:18                                                                         ` SF Markus Elfring
2017-11-28 20:18                                                                         ` SF Markus Elfring
2017-11-28 20:25                                                                         ` Takashi Iwai
2017-11-28 20:25                                                                           ` Takashi Iwai
2017-11-28 20:32                                                                           ` SF Markus Elfring
2017-11-28 20:32                                                                             ` SF Markus Elfring
2017-11-28 20:32                                                                             ` SF Markus Elfring
2017-11-29 10:34               ` SF Markus Elfring
2017-11-29 10:34                 ` SF Markus Elfring
2017-11-29 10:34                 ` SF Markus Elfring
2017-11-28 12:33             ` [alsa-devel] " Ondrej Zary
2017-11-28 12:33               ` Ondrej Zary
2017-11-28 13:10               ` SF Markus Elfring
2017-11-28 13:10                 ` [alsa-devel] " SF Markus Elfring
2017-11-28 13:10                 ` SF Markus Elfring

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2cbef557-5f89-c630-e108-14ef2ce6b41a@users.sourceforge.net \
    --to=elfring@users.sourceforge.net \
    --cc=alsa-devel@alsa-project.org \
    --cc=arvind.yadav.cs@gmail.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=o-takashi@sakamocchi.jp \
    --cc=tiwai@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.