* [PATCH] streamline_config.pl: use references rather than copied data structures
@ 2010-05-27 11:42 Toralf Förster
2010-05-27 13:07 ` Steven Rostedt
0 siblings, 1 reply; 5+ messages in thread
From: Toralf Förster @ 2010-05-27 11:42 UTC (permalink / raw)
To: rostedt; +Cc: zippel, linux-kbuild, linux-kernel
IMHO there's no need for a copy-by-value
Signed-off-by: Toralf Foerster <toralf.foerster@gmx.de>
---
scripts/kconfig/streamline_config.pl | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/scripts/kconfig/streamline_config.pl
b/scripts/kconfig/streamline_config.pl
index afbd54a..574e68f 100644
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -406,14 +406,14 @@ close(CIN);
loop:
foreach my $module (keys(%modules)) {
if (defined($objects{$module})) {
- my @arr = @{$objects{$module}};
- foreach my $conf (@arr) {
+ my $arr = $objects{$module};
+ foreach my $conf (@{$arr}) {
if (defined($setconfigs{$conf})) {
next loop;
}
}
print STDERR "module $module did not have configs";
- foreach my $conf (@arr) {
+ foreach my $conf (@{$arr}) {
print STDERR " " , $conf;
}
print STDERR "\n";
--
1.6.4.4
--
MfG/Sincerely
Toralf Förster
pgp finger print: 7B1A 07F4 EC82 0F90 D4C2 8936 872A E508 7DB6 9DA3
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] streamline_config.pl: use references rather than copied data structures
2010-05-27 11:42 [PATCH] streamline_config.pl: use references rather than copied data structures Toralf Förster
@ 2010-05-27 13:07 ` Steven Rostedt
2010-05-27 14:24 ` Toralf Förster
0 siblings, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2010-05-27 13:07 UTC (permalink / raw)
To: Toralf Förster; +Cc: zippel, linux-kbuild, linux-kernel
On Thu, 2010-05-27 at 13:42 +0200, Toralf Förster wrote:
> IMHO there's no need for a copy-by-value
>
Hi Toralf,
I'm sorry but "Your Humble Opinion" is not rational to make changes.
A change to the code must have a reason other than opinion. You must be
able to show that there is actual benefit for a change, or at least
explain it.
Every time a change is made, there is potential for a new bug to be
introduced (especially in Perl). So, unless there's actual need for a
change, don't make one.
This is not performance critical code. If you are fixing a bug, or
adding a feature, then sure. But I really don't want changes that do
nothing but skin the cat a different way.
Thanks,
-- Steve
> Signed-off-by: Toralf Foerster <toralf.foerster@gmx.de>
> ---
> scripts/kconfig/streamline_config.pl | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/kconfig/streamline_config.pl
> b/scripts/kconfig/streamline_config.pl
> index afbd54a..574e68f 100644
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -406,14 +406,14 @@ close(CIN);
> loop:
> foreach my $module (keys(%modules)) {
> if (defined($objects{$module})) {
> - my @arr = @{$objects{$module}};
> - foreach my $conf (@arr) {
> + my $arr = $objects{$module};
> + foreach my $conf (@{$arr}) {
> if (defined($setconfigs{$conf})) {
> next loop;
> }
> }
> print STDERR "module $module did not have configs";
> - foreach my $conf (@arr) {
> + foreach my $conf (@{$arr}) {
> print STDERR " " , $conf;
> }
> print STDERR "\n";
> --
> 1.6.4.4
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] streamline_config.pl: use references rather than copied data structures
2010-05-27 13:07 ` Steven Rostedt
@ 2010-05-27 14:24 ` Toralf Förster
2010-05-27 18:50 ` Steven Rostedt
2010-05-27 19:52 ` Steven Rostedt
0 siblings, 2 replies; 5+ messages in thread
From: Toralf Förster @ 2010-05-27 14:24 UTC (permalink / raw)
To: rostedt; +Cc: zippel, linux-kbuild, linux-kernel
Steven Rostedt wrote at 15:07:29
> On Thu, 2010-05-27 at 13:42 +0200, Toralf Förster wrote:
> > IMHO there's no need for a copy-by-value
>
> Hi Toralf,
>
> I'm sorry but "Your Humble Opinion" is not rational to make changes.
>
> A change to the code must have a reason other than opinion. You must be
> able to show that there is actual benefit for a change, or at least
> explain it.
>
> Every time a change is made, there is potential for a new bug to be
> introduced (especially in Perl). So, unless there's actual need for a
> change, don't make one.
>
> This is not performance critical code. If you are fixing a bug, or
> adding a feature, then sure. But I really don't want changes that do
> nothing but skin the cat a different way.
>
> Thanks,
>
> -- Steve
Understood and accepted.
What's about the following 2 snippets (a trivial typo and an attempt to
prevent a possible trap) ? :
diff --git a/scripts/kconfig/streamline_config.pl
b/scripts/kconfig/streamline_config.pl
index afbd54a..3f54911 100644
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -242,7 +242,7 @@ foreach my $makefile (@makefiles) {
foreach my $obj (split /\s+/,$objs) {
$obj =~ s/-/_/g;
if ($obj =~ /(.*)\.o$/) {
- # Objects may bes enabled by more than one config.
+ # Objects may be enabled by more than one config.
# Store configs in an array.
my @arr;
diff --git a/scripts/kconfig/streamline_config.pl
b/scripts/kconfig/streamline_config.pl
index afbd54a..9726946 100644
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -307,7 +307,7 @@ close (LIN);
my %configs;
foreach my $module (keys(%modules)) {
if (defined($objects{$module})) {
- @arr = @{$objects{$module}};
+ my @arr = @{$objects{$module}};
foreach my $conf (@arr) {
$configs{$conf} = $module;
}
--
MfG/Sincerely
Toralf Förster
pgp finger print: 7B1A 07F4 EC82 0F90 D4C2 8936 872A E508 7DB6 9DA3
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] streamline_config.pl: use references rather than copied data structures
2010-05-27 14:24 ` Toralf Förster
@ 2010-05-27 18:50 ` Steven Rostedt
2010-05-27 19:52 ` Steven Rostedt
1 sibling, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2010-05-27 18:50 UTC (permalink / raw)
To: Toralf Förster; +Cc: zippel, linux-kbuild, linux-kernel
On Thu, 2010-05-27 at 16:24 +0200, Toralf Förster wrote:
> Steven Rostedt wrote at 15:07:29
> > On Thu, 2010-05-27 at 13:42 +0200, Toralf Förster wrote:
> Understood and accepted.
>
> What's about the following 2 snippets (a trivial typo and an attempt to
> prevent a possible trap) ? :
>
OK, this is better :-)
-- Steve
>
> diff --git a/scripts/kconfig/streamline_config.pl
> b/scripts/kconfig/streamline_config.pl
> index afbd54a..3f54911 100644
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -242,7 +242,7 @@ foreach my $makefile (@makefiles) {
> foreach my $obj (split /\s+/,$objs) {
> $obj =~ s/-/_/g;
> if ($obj =~ /(.*)\.o$/) {
> - # Objects may bes enabled by more than one config.
> + # Objects may be enabled by more than one config.
> # Store configs in an array.
> my @arr;
>
>
>
> diff --git a/scripts/kconfig/streamline_config.pl
> b/scripts/kconfig/streamline_config.pl
> index afbd54a..9726946 100644
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -307,7 +307,7 @@ close (LIN);
> my %configs;
> foreach my $module (keys(%modules)) {
> if (defined($objects{$module})) {
> - @arr = @{$objects{$module}};
> + my @arr = @{$objects{$module}};
> foreach my $conf (@arr) {
> $configs{$conf} = $module;
> }
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] streamline_config.pl: use references rather than copied data structures
2010-05-27 14:24 ` Toralf Förster
2010-05-27 18:50 ` Steven Rostedt
@ 2010-05-27 19:52 ` Steven Rostedt
1 sibling, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2010-05-27 19:52 UTC (permalink / raw)
To: Toralf Förster; +Cc: zippel, linux-kbuild, linux-kernel
On Thu, 2010-05-27 at 16:24 +0200, Toralf Förster wrote:
> Understood and accepted.
>
> What's about the following 2 snippets (a trivial typo and an attempt to
> prevent a possible trap) ? :
>
Can you make a proper patch out of this (with signed-off-by as well).
Also your patches have major whitespace damage. Could you fix your mail
client.
Thanks,
-- Steve
>
> diff --git a/scripts/kconfig/streamline_config.pl
> b/scripts/kconfig/streamline_config.pl
> index afbd54a..3f54911 100644
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -242,7 +242,7 @@ foreach my $makefile (@makefiles) {
> foreach my $obj (split /\s+/,$objs) {
> $obj =~ s/-/_/g;
> if ($obj =~ /(.*)\.o$/) {
> - # Objects may bes enabled by more than one config.
> + # Objects may be enabled by more than one config.
> # Store configs in an array.
> my @arr;
>
>
>
> diff --git a/scripts/kconfig/streamline_config.pl
> b/scripts/kconfig/streamline_config.pl
> index afbd54a..9726946 100644
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -307,7 +307,7 @@ close (LIN);
> my %configs;
> foreach my $module (keys(%modules)) {
> if (defined($objects{$module})) {
> - @arr = @{$objects{$module}};
> + my @arr = @{$objects{$module}};
> foreach my $conf (@arr) {
> $configs{$conf} = $module;
> }
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-05-27 19:52 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-27 11:42 [PATCH] streamline_config.pl: use references rather than copied data structures Toralf Förster
2010-05-27 13:07 ` Steven Rostedt
2010-05-27 14:24 ` Toralf Förster
2010-05-27 18:50 ` Steven Rostedt
2010-05-27 19:52 ` Steven Rostedt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox