Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1476607 > unrolled thread

[PATCH 00/21] ACPI-video: Fine-tuning for several function implementations

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 00/21] ACPI-video: Fine-tuning for several function  implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-05 18:50 +0200
    [PATCH 01/21] ACPI-video: Use kmalloc_array() in  acpi_video_get_levels() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-05 18:50 +0200
    [PATCH 02/21] ACPI-video: Return directly after a failed device query SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-05 18:50 +0200
    [PATCH 08/21] ACPI-video: Improve a size determination in  acpi_video_bus_add() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-05 19:00 +0200
    [PATCH 07/21] ACPI-video: Rename jump labels in acpi_video_bus_add() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-05 19:00 +0200
    [PATCH 11/21] ACPI-video: Rename jump labels in  acpi_video_bus_add_notify_handler() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-05 19:00 +0200
    [PATCH 14/21] ACPI-video: Improve a size determination in  acpi_video_device_enumerate() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-05 19:10 +0200
    [PATCH 15/21] ACPI-video: Delete an unnecessary initialisation in  acpi_video_device_enumerate() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-05 19:10 +0200
    [PATCH 12/21] ACPI-video: Delete unnecessary if statement in  acpi_video_switch_brightness() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-05 19:10 +0200
    [PATCH 18/21] ACPI-video: Rename jump labels in  acpi_video_init_brightness() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-05 19:10 +0200
    [PATCH 13/21] ACPI-video: Improve a jump target in  acpi_video_switch_brightness() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-05 19:10 +0200
    [PATCH 09/21] ACPI-video: Rename jump labels in acpi_video_register() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-05 19:10 +0200
    [PATCH 16/21] ACPI-video: Rename jump labels in  acpi_video_device_enumerate() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-05 19:10 +0200
    [PATCH 19/21] ACPI-video: Rename a jump label in  acpi_video_device_lcd_query_levels() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-05 19:10 +0200
    [PATCH 21/21] ACPI-video: Improve a size determination in  acpi_video_bus_get_one_device() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-05 19:10 +0200
    [PATCH 10/21] ACPI-video: Return directly after a failed  input_allocate_device() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-05 19:10 +0200
    [PATCH 17/21] ACPI-video: Delete an unnecessary initialisation in  acpi_video_init_brightness() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-05 19:10 +0200
    [PATCH 20/21] ACPI-video: Improve a size determination in  acpi_video_dev_register_backlight() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-05 19:10 +0200
    [PATCH 04/21] ACPI-video: Rename jump labels in  acpi_video_get_levels() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-05 19:30 +0200
    [PATCH 06/21] ACPI-video: Move four assignments in  acpi_video_get_levels() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-05 19:30 +0200
    [PATCH 05/21] ACPI-video: Delete an unnecessary initialisation in  acpi_video_get_levels() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-05 19:30 +0200
    [PATCH 03/21] ACPI-video: Delete an error message for a failed  kzalloc() call SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-05 19:40 +0200
    Re: [PATCH 00/21] ACPI-video: Fine-tuning for several function implementations "Rafael J. Wysocki" <rafael@kernel.org> - 2016-09-05 23:50 +0200
      Re: ACPI-video: Fine-tuning for several function implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-06 05:30 +0200
        Re: ACPI-video: Fine-tuning for several function implementations "Rafael J. Wysocki" <rafael@kernel.org> - 2016-09-06 13:30 +0200
          Re: ACPI-video: Fine-tuning for several function implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-06 16:20 +0200
            Re: ACPI-video: Fine-tuning for several function implementations "Rafael J. Wysocki" <rafael@kernel.org> - 2016-09-06 23:10 +0200
              Re: ACPI-video: Fine-tuning for several function implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2016-09-07 08:50 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1476820 — [PATCH 05/21] ACPI-video: Delete an unnecessary initialisation in acpi_video_get_levels()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 5 Sep 2016 14:23:55 +0200

The local variable "br" will be set to an appropriate value a bit later.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/acpi/acpi_video.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index 82987db..0799865 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -760,7 +760,7 @@ int acpi_video_get_levels(struct acpi_device *device,
 	union acpi_object *obj = NULL;
 	int i, max_level = 0, count = 0, level_ac_battery = 0;
 	union acpi_object *o;
-	struct acpi_video_device_brightness *br = NULL;
+	struct acpi_video_device_brightness *br;
 	int result = 0;
 	u32 value;
 
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1476830 — [PATCH 03/21] ACPI-video: Delete an error message for a failed kzalloc() call

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 5 Sep 2016 13:57:33 +0200

Omit an extra message for a memory allocation failure in this function.

Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/acpi/acpi_video.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index 8f0807a..420d125 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -778,7 +778,6 @@ int acpi_video_get_levels(struct acpi_device *device,
 
 	br = kzalloc(sizeof(*br), GFP_KERNEL);
 	if (!br) {
-		printk(KERN_ERR "can't allocate memory\n");
 		result = -ENOMEM;
 		goto out;
 	}
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1476968 — Re: [PATCH 00/21] ACPI-video: Fine-tuning for several function implementations

On Mon, Sep 5, 2016 at 6:42 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 5 Sep 2016 18:22:11 +0200
>
> Several update suggestions were taken into account
> from static source code analysis.
>
> Markus Elfring (21):
>   Use kmalloc_array() in acpi_video_get_levels()
>   Return directly after a failed device query
>   Delete an error message for a failed kzalloc() call
>   Rename jump labels in acpi_video_get_levels()
>   Delete an unnecessary initialisation in acpi_video_get_levels()
>   Move four assignments in acpi_video_get_levels()
>   Rename jump labels in acpi_video_bus_add()
>   Improve a size determination in acpi_video_bus_add()
>   Rename jump labels in acpi_video_register()
>   Return directly after a failed input_allocate_device()
>   Rename jump labels in acpi_video_bus_add_notify_handler()
>   Delete unnecessary if statement in acpi_video_switch_brightness()
>   Improve a jump target in acpi_video_switch_brightness()
>   Improve a size determination in acpi_video_device_enumerate()
>   Delete an unnecessary initialisation in acpi_video_device_enumerate()
>   Rename jump labels in acpi_video_device_enumerate()
>   Delete an unnecessary initialisation in acpi_video_init_brightness()
>   Rename jump labels in acpi_video_init_brightness()
>   Rename a jump label in acpi_video_device_lcd_query_levels()
>   Improve a size determination in acpi_video_dev_register_backlight()
>   Improve a size determination in acpi_video_bus_get_one_device()

I'd prefer this to be combined into fewer patches that each will
address several issues of one type, ie. put all label renames into one
patch, all size determination improvements into another one and so on.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1477045 — Re: ACPI-video: Fine-tuning for several function implementations

> I'd prefer this to be combined into fewer patches
> that each will address several issues of one type,

I understand your concern a bit in principle.


> ie. put all label renames into one patch,

Are any of my update suggestions controversial here?


> all size determination improvements into another one and so on.

I am unsure about the acceptance for the selected software change opportunities.
So I chose a very specific patch granularity intentionally.

I tend to provide some change ideas for each affected function
implementation individually. I imagine that this way should support
the recombination of update steps to some degree already, shouldn't it?

Regards,
Markus

[toc] | [prev] | [next] | [standalone]

#1477288 — Re: ACPI-video: Fine-tuning for several function implementations

On Tue, Sep 6, 2016 at 5:28 AM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
>> I'd prefer this to be combined into fewer patches
>> that each will address several issues of one type,
>
> I understand your concern a bit in principle.
>
>
>> ie. put all label renames into one patch,
>
> Are any of my update suggestions controversial here?

Well, the label renames have a little value in general IMO, but that
depends on a particular case.

Anyway, if there's something I don't like in particular, I'll let you know.

>> all size determination improvements into another one and so on.
>
> I am unsure about the acceptance for the selected software change opportunities.
> So I chose a very specific patch granularity intentionally.
>
> I tend to provide some change ideas for each affected function
> implementation individually. I imagine that this way should support
> the recombination of update steps to some degree already, shouldn't it?

However, it's a pain to review 20 patches if you could review 4 instead.

Please take the reviewers' time into consideration too.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1477455 — Re: ACPI-video: Fine-tuning for several function implementations

> Anyway, if there's something I don't like in particular, I'll let you know.

Thanks for your general interest.

I hope that occasional disagreements can be resolved in constructive ways.


> However, it's a pain to review 20 patches if you could review 4 instead.

Are there any more possibilities to improve the convenience for this
change review process with advanced tools?


> Please take the reviewers' time into consideration too.

I am trying this to some degree.

But I guess that it is hard to do something about corresponding efforts
when various contributors can easily spot many software update opportunities
in the discussed source files, isn't it?

Regards,
Markus

[toc] | [prev] | [next] | [standalone]

#1477823 — Re: ACPI-video: Fine-tuning for several function implementations

On Tue, Sep 6, 2016 at 4:10 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
>> Anyway, if there's something I don't like in particular, I'll let you know.
>
> Thanks for your general interest.
>
> I hope that occasional disagreements can be resolved in constructive ways.
>
>
>> However, it's a pain to review 20 patches if you could review 4 instead.
>
> Are there any more possibilities to improve the convenience for this
> change review process with advanced tools?
>
>
>> Please take the reviewers' time into consideration too.
>
> I am trying this to some degree.
>
> But I guess that it is hard to do something about corresponding efforts
> when various contributors can easily spot many software update opportunities
> in the discussed source files, isn't it?

OK, look.

Your patches happen to modify code maintained by me.  From my
perspective the value of the changes made by them is marginal.
Nevertheless, I might take them if you made my life somewhat easier,
so I've tried to tell you politely how to do that.

If you're not willing to do it, though, this is where it ends.  And
attempts to convince me that I may not want my life to be easier after
all are not likely to succeed.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1477998 — Re: ACPI-video: Fine-tuning for several function implementations

> Your patches happen to modify code maintained by me.  From my
> perspective the value of the changes made by them is marginal.

Thanks for another bit of interesting information.


> Nevertheless, I might take them if you made my life somewhat easier,

I am also looking for further approaches to help you there.


> so I've tried to tell you politely how to do that.

This feedback is generally fine.


> If you're not willing to do it,

My willingness is depending on also some factors.


> though, this is where it ends.

I hope that a bit more clarification can improve the situation.


> And attempts to convince me that I may not want my life to be easier
> after all are not likely to succeed.

We usually want that life will become more comfortable.

I chose to contribute something to Linux source files for this purpose.
My knowledge evolved in the way that I am using some tools for
static source code analysis. Such advanced tools can point various
change opportunities out. I picked a few special search patterns up.
It happened then that hundreds of source files were found which contain
update candidates. I am trying to inform the corresponding developers
about improvement possibilities in affected systems.


Further challenges are relevant then as usual.

* Handling of the search process and their results

* Communication between contributors


Search patterns can occasionally be categorised as "too special".
The software technology contains also the risk for showing "false positives".

The reactions of code reviewers are varying between rejection and acceptance.
Now I would like to determine again which details of the proposed changes
have got a higher chance for acceptance.

The discussed concrete patch series is just another example for usual
difficulties or more interesting software development challenges.
I hope that they can be resolved in a systematic way.
I sent analysis results as a series of small software updates. I find
it important to understand them also in the way that they belong to
software design patterns. I can imagine that it is harder to recognise
the involved patterns from the presented combination of update steps.

Would you like to check and clarify these patterns once more
before the desired improvements will happen (in a software area you maintain)?


So there are further constraints to consider. My software development experience
leaded me to a very specific kind of patch granularity here.
My software development interest evolved also in the way that I dared
to fiddle with the source files "drivers/acpi/processor_perflib.c"
and "drivers/acpi/processor_throttling.c" yesterday.
The consequence is that I would to publish a corresponding series
of 30 update steps for integration into another source code repository.
It seems that I need to wait a bit more for the next contribution attempt
before the change acceptance will fit to such an approach.

Regards,
Markus

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web