Re: GPU-DRM-STI: Fine-tuning for some function implementations

From: Sean Paul
Date: Sat May 06 2017 - 11:25:42 EST


On Sat, May 06, 2017 at 03:54:51PM +0200, SF Markus Elfring wrote:
> > Generally speaking, I don't care about checkpatch/cocci changes that aren't tested.
>
> I find this view interesting only to some degree.

We're bordering on becoming unproductive here, but I'll try once more.

>
> 1. I suggest to combine a few functions into fewer ones.
> * Do you spot any programming mistakes in these concrete cases?

Not in the patches I skimmed. However, your history of breaking code tells me that
there have been mistakes missed in the past. As such, I'm not willing to take
untested code from you that does not change functionality at the risk of
breaking something that is currently working. This is non-negotiable.

> * Can such code reduction result into desired effects?
>
> 2. I propose to use the function âseq_putcâ at more source code places.
> * Do you really find any previous system test approaches insufficient around
> such a Linux feature?
> * Does the programming interface âseq_putsâ provide any properties
> that you prefer over the other one for the sequence output of single characters?
> http://elixir.free-electrons.com/linux/v4.11/source/fs/seq_file.c#L664
>

As I said before, if you test it, I'll consider it. If you are unwilling to test
your changes, I'm unwilling to apply them. I'm not interested in double checking
all of your work, and fixing your bugs for no functional benefit.

>
> > With your changes, we don't have this upside.
>
> How do you think about to pick spelling corrections up for two comment lines?

Well, it wouldn't break anything, so that's positive. As I said in my last
email, these types of changes are perfect for new contributors to get started
with kernel development. I find less value in these patches if they're from
someone seemingly trying to rack up patch count.

Sean


>
> Regards,
> Markus

--
Sean Paul, Software Engineer, Google / Chromium OS