[PATCH 1/2] localmodconfig: Fix parsing of Kconfig "source" statements

[PATCH 1/2] localmodconfig: Fix parsing of Kconfig "source" statements

[Benjamin Poirier]

The parameter of Kconfig "source" statements does not need to be quoted.
The current regex causes many kconfig files to be skipped and hence,
dependencies to be missed.

Also fix the whitespace repeat count.

Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
---
 scripts/kconfig/streamline_config.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index f3d3fb4..7036ae3 100755
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -188,7 +188,7 @@ sub read_kconfig {
 	$cont = 0;
 
 	# collect any Kconfig sources
-	if (/^source\s*"(.*)"/) {
+	if (/^source\s+"?([^"]+)/) {
 	    my $kconfig = $1;
 	    # prevent reading twice.
 	    if (!defined($read_kconfigs{$kconfig})) {
-- 
2.7.2

[PATCH 2/2] localmodconfig: Reset certificate paths

[Benjamin Poirier]

When using `make localmodconfig` and friends, if the input config comes
from a kernel that was built in a different environment (for example, the
canonical case of using localmodconfig to trim a distribution kernel
config) the key files for module signature checking will not be available
and should be regenerated or omitted. Otherwise, the user will be faced
with annoying errors when trying to build with the generated .config:

make[1]: *** No rule to make target 'keyring.crt', needed by 'certs/x509_certificate_list'.  Stop.
Makefile:1576: recipe for target 'certs/' failed

Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
---
 scripts/kconfig/streamline_config.pl | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index 7036ae3..514735d 100755
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -610,6 +610,40 @@ foreach my $line (@config_file) {
 	next;
     }
 
+    if (/CONFIG_MODULE_SIG_KEY="(.+)"/) {
+        my $orig_cert = $1;
+        my $default_cert = "certs/signing_key.pem";
+
+        # Check that the logic in this script still matches the one in Kconfig
+        if (!defined($depends{"MODULE_SIG_KEY"}) ||
+            $depends{"MODULE_SIG_KEY"} !~ /"\Q$default_cert\E"/) {
+            die "Assertion failure, update needed";
+        }
+
+        if ($orig_cert ne $default_cert && ! -f $orig_cert) {
+            print STDERR "Module signature verification enabled but ",
+                "module signing key \"$orig_cert\" not found. Resetting ",
+                "signing key to default value.\n";
+            print "CONFIG_MODULE_SIG_KEY=\"$default_cert\"\n";
+        } else {
+            print;
+        }
+        next;
+    }
+
+    if (/CONFIG_SYSTEM_TRUSTED_KEYS="(.+)"/) {
+        my $orig_keys = $1;
+
+        if (! -f $orig_keys) {
+            print STDERR "System keyring enabled but keys \"$orig_keys\" ",
+                "not found. Resetting keys to default value.\n";
+            print "CONFIG_SYSTEM_TRUSTED_KEYS=\"\"\n";
+        } else {
+            print;
+        }
+        next;
+    }
+
     if (/^(CONFIG.*)=(m|y)/) {
 	if (defined($configs{$1})) {
 	    if ($localyesconfig) {
-- 
2.7.2

Re: [PATCH 2/2] localmodconfig: Reset certificate paths

[joeyli]

On Sat, Apr 02, 2016 at 10:55:22AM -0700, Benjamin Poirier wrote:
> When using `make localmodconfig` and friends, if the input config comes
> from a kernel that was built in a different environment (for example, the
> canonical case of using localmodconfig to trim a distribution kernel
> config) the key files for module signature checking will not be available
> and should be regenerated or omitted. Otherwise, the user will be faced
> with annoying errors when trying to build with the generated .config:
> 
> make[1]: *** No rule to make target 'keyring.crt', needed by 'certs/x509_certificate_list'.  Stop.
> Makefile:1576: recipe for target 'certs/' failed
> 
> Signed-off-by: Benjamin Poirier <bpoirier@suse.com>

Tested-by: Lee, Chun-Yi <jlee@suse.com>


Regards
Joey Lee

> ---
>  scripts/kconfig/streamline_config.pl | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> index 7036ae3..514735d 100755
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -610,6 +610,40 @@ foreach my $line (@config_file) {
>  	next;
>      }
>  
> +    if (/CONFIG_MODULE_SIG_KEY="(.+)"/) {
> +        my $orig_cert = $1;
> +        my $default_cert = "certs/signing_key.pem";
> +
> +        # Check that the logic in this script still matches the one in Kconfig
> +        if (!defined($depends{"MODULE_SIG_KEY"}) ||
> +            $depends{"MODULE_SIG_KEY"} !~ /"\Q$default_cert\E"/) {
> +            die "Assertion failure, update needed";
> +        }
> +
> +        if ($orig_cert ne $default_cert && ! -f $orig_cert) {
> +            print STDERR "Module signature verification enabled but ",
> +                "module signing key \"$orig_cert\" not found. Resetting ",
> +                "signing key to default value.\n";
> +            print "CONFIG_MODULE_SIG_KEY=\"$default_cert\"\n";
> +        } else {
> +            print;
> +        }
> +        next;
> +    }
> +
> +    if (/CONFIG_SYSTEM_TRUSTED_KEYS="(.+)"/) {
> +        my $orig_keys = $1;
> +
> +        if (! -f $orig_keys) {
> +            print STDERR "System keyring enabled but keys \"$orig_keys\" ",
> +                "not found. Resetting keys to default value.\n";
> +            print "CONFIG_SYSTEM_TRUSTED_KEYS=\"\"\n";
> +        } else {
> +            print;
> +        }
> +        next;
> +    }
> +
>      if (/^(CONFIG.*)=(m|y)/) {
>  	if (defined($configs{$1})) {
>  	    if ($localyesconfig) {
> -- 
> 2.7.2

Re: [PATCH 2/2] localmodconfig: Reset certificate paths

[Steven Rostedt]

On Sat,  2 Apr 2016 10:55:22 -0700
Benjamin Poirier <bpoirier@suse.com> wrote:

> When using `make localmodconfig` and friends, if the input config comes
> from a kernel that was built in a different environment (for example, the
> canonical case of using localmodconfig to trim a distribution kernel
> config) the key files for module signature checking will not be available
> and should be regenerated or omitted. Otherwise, the user will be faced
> with annoying errors when trying to build with the generated .config:
> 
> make[1]: *** No rule to make target 'keyring.crt', needed by 'certs/x509_certificate_list'.  Stop.
> Makefile:1576: recipe for target 'certs/' failed
> 
> Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
> ---
>  scripts/kconfig/streamline_config.pl | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> index 7036ae3..514735d 100755
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -610,6 +610,40 @@ foreach my $line (@config_file) {
>  	next;
>      }
>  
> +    if (/CONFIG_MODULE_SIG_KEY="(.+)"/) {
> +        my $orig_cert = $1;
> +        my $default_cert = "certs/signing_key.pem";
> +
> +        # Check that the logic in this script still matches the one in Kconfig
> +        if (!defined($depends{"MODULE_SIG_KEY"}) ||
> +            $depends{"MODULE_SIG_KEY"} !~ /"\Q$default_cert\E"/) {
> +            die "Assertion failure, update needed";

Instead of dieing here, what about just going back to the current
behavior, and ignore the sig keys?

-- Steve

> +        }
> +
> +        if ($orig_cert ne $default_cert && ! -f $orig_cert) {
> +            print STDERR "Module signature verification enabled but ",
> +                "module signing key \"$orig_cert\" not found. Resetting ",
> +                "signing key to default value.\n";
> +            print "CONFIG_MODULE_SIG_KEY=\"$default_cert\"\n";
> +        } else {
> +            print;
> +        }
> +        next;
> +    }
> +
> +    if (/CONFIG_SYSTEM_TRUSTED_KEYS="(.+)"/) {
> +        my $orig_keys = $1;
> +
> +        if (! -f $orig_keys) {
> +            print STDERR "System keyring enabled but keys \"$orig_keys\" ",
> +                "not found. Resetting keys to default value.\n";
> +            print "CONFIG_SYSTEM_TRUSTED_KEYS=\"\"\n";
> +        } else {
> +            print;
> +        }
> +        next;
> +    }
> +
>      if (/^(CONFIG.*)=(m|y)/) {
>  	if (defined($configs{$1})) {
>  	    if ($localyesconfig) {

Re: [PATCH 2/2] localmodconfig: Reset certificate paths

[Benjamin Poirier]

On 2016/04/26 10:02, Steven Rostedt wrote:
> On Sat,  2 Apr 2016 10:55:22 -0700
> Benjamin Poirier <bpoirier@suse.com> wrote:
> 
> > When using `make localmodconfig` and friends, if the input config comes
> > from a kernel that was built in a different environment (for example, the
> > canonical case of using localmodconfig to trim a distribution kernel
> > config) the key files for module signature checking will not be available
> > and should be regenerated or omitted. Otherwise, the user will be faced
> > with annoying errors when trying to build with the generated .config:
> > 
> > make[1]: *** No rule to make target 'keyring.crt', needed by 'certs/x509_certificate_list'.  Stop.
> > Makefile:1576: recipe for target 'certs/' failed
> > 
> > Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
> > ---
> >  scripts/kconfig/streamline_config.pl | 34 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 34 insertions(+)
> > 
> > diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> > index 7036ae3..514735d 100755
> > --- a/scripts/kconfig/streamline_config.pl
> > +++ b/scripts/kconfig/streamline_config.pl
> > @@ -610,6 +610,40 @@ foreach my $line (@config_file) {
> >  	next;
> >      }
> >  
> > +    if (/CONFIG_MODULE_SIG_KEY="(.+)"/) {
> > +        my $orig_cert = $1;
> > +        my $default_cert = "certs/signing_key.pem";
> > +
> > +        # Check that the logic in this script still matches the one in Kconfig
> > +        if (!defined($depends{"MODULE_SIG_KEY"}) ||
> > +            $depends{"MODULE_SIG_KEY"} !~ /"\Q$default_cert\E"/) {
> > +            die "Assertion failure, update needed";
> 
> Instead of dieing here, what about just going back to the current
> behavior, and ignore the sig keys?

I was concerned that the warning may go unnoticed but I think you're right. It
is the same kind of concern between a BUG() or a WARN_ON(). In this case it
certainly is possible to keep going and ignore the certificate check, as you
suggest.

[PATCH v2] localmodconfig: Reset certificate paths

[Benjamin Poirier]

When using `make localmodconfig` and friends, if the input config comes
from a kernel that was built in a different environment (for example, the
canonical case of using localmodconfig to trim a distribution kernel
config) the key files for module signature checking will not be available
and should be regenerated or omitted. Otherwise, the user will be faced
with annoying errors when trying to build with the generated .config:

make[1]: *** No rule to make target 'keyring.crt', needed by 'certs/x509_certificate_list'.  Stop.
Makefile:1576: recipe for target 'certs/' failed

Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
---
 scripts/kconfig/streamline_config.pl | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index 01b53ec..95a6f2b 100755
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -610,6 +610,40 @@ foreach my $line (@config_file) {
 	next;
     }
 
+    if (/CONFIG_MODULE_SIG_KEY="(.+)"/) {
+        my $orig_cert = $1;
+        my $default_cert = "certs/signing_key.pem";
+
+        # Check that the logic in this script still matches the one in Kconfig
+        if (!defined($depends{"MODULE_SIG_KEY"}) ||
+            $depends{"MODULE_SIG_KEY"} !~ /"\Q$default_cert\E"/) {
+            print STDERR "WARNING: MODULE_SIG_KEY assertion failure, ",
+                "update needed to ", __FILE__, " line ", __LINE__, "\n";
+            print;
+        } elsif ($orig_cert ne $default_cert && ! -f $orig_cert) {
+            print STDERR "Module signature verification enabled but ",
+                "module signing key \"$orig_cert\" not found. Resetting ",
+                "signing key to default value.\n";
+            print "CONFIG_MODULE_SIG_KEY=\"$default_cert\"\n";
+        } else {
+            print;
+        }
+        next;
+    }
+
+    if (/CONFIG_SYSTEM_TRUSTED_KEYS="(.+)"/) {
+        my $orig_keys = $1;
+
+        if (! -f $orig_keys) {
+            print STDERR "System keyring enabled but keys \"$orig_keys\" ",
+                "not found. Resetting keys to default value.\n";
+            print "CONFIG_SYSTEM_TRUSTED_KEYS=\"\"\n";
+        } else {
+            print;
+        }
+        next;
+    }
+
     if (/^(CONFIG.*)=(m|y)/) {
 	if (defined($configs{$1})) {
 	    if ($localyesconfig) {
-- 
2.7.4

Re: [PATCH v2] localmodconfig: Reset certificate paths

[Steven Rostedt]

On Tue, 26 Apr 2016 11:52:01 -0700
Benjamin Poirier <bpoirier@suse.com> wrote:

> When using `make localmodconfig` and friends, if the input config comes
> from a kernel that was built in a different environment (for example, the
> canonical case of using localmodconfig to trim a distribution kernel
> config) the key files for module signature checking will not be available
> and should be regenerated or omitted. Otherwise, the user will be faced
> with annoying errors when trying to build with the generated .config:
> 
> make[1]: *** No rule to make target 'keyring.crt', needed by 'certs/x509_certificate_list'.  Stop.
> Makefile:1576: recipe for target 'certs/' failed
> 
> Signed-off-by: Benjamin Poirier <bpoirier@suse.com>

Applied, thanks!

-- Steve

Re: [PATCH 1/2] localmodconfig: Fix parsing of Kconfig "source" statements

[joeyli]

On Sat, Apr 02, 2016 at 10:55:21AM -0700, Benjamin Poirier wrote:
> The parameter of Kconfig "source" statements does not need to be quoted.
> The current regex causes many kconfig files to be skipped and hence,
> dependencies to be missed.
> 
> Also fix the whitespace repeat count.
> 
> Signed-off-by: Benjamin Poirier <bpoirier@suse.com>

Tested-by: Lee, Chun-Yi <jlee@suse.com>

> ---
>  scripts/kconfig/streamline_config.pl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> index f3d3fb4..7036ae3 100755
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -188,7 +188,7 @@ sub read_kconfig {
>  	$cont = 0;
>  
>  	# collect any Kconfig sources
> -	if (/^source\s*"(.*)"/) {
> +	if (/^source\s+"?([^"]+)/) {
>  	    my $kconfig = $1;
>  	    # prevent reading twice.
>  	    if (!defined($read_kconfigs{$kconfig})) {
> -- 
> 2.7.2

Regards
Joey Lee

Re: [PATCH 1/2] localmodconfig: Fix parsing of Kconfig "source" statements

[Steven Rostedt]

On Fri, 8 Apr 2016 22:59:02 +0800
joeyli <jlee@suse.com> wrote:

> On Sat, Apr 02, 2016 at 10:55:21AM -0700, Benjamin Poirier wrote:
> > The parameter of Kconfig "source" statements does not need to be quoted.
> > The current regex causes many kconfig files to be skipped and hence,
> > dependencies to be missed.
> > 
> > Also fix the whitespace repeat count.
> > 
> > Signed-off-by: Benjamin Poirier <bpoirier@suse.com>  
> 
> Tested-by: Lee, Chun-Yi <jlee@suse.com>
> 

Applied.

Thanks!

-- Steve

Re: [PATCH 1/2] localmodconfig: Fix parsing of Kconfig "source" statements

[Steven Rostedt]

On Sat,  2 Apr 2016 10:55:21 -0700
Benjamin Poirier <bpoirier@suse.com> wrote:

> The parameter of Kconfig "source" statements does not need to be quoted.
> The current regex causes many kconfig files to be skipped and hence,
> dependencies to be missed.
> 
> Also fix the whitespace repeat count.
> 
> Signed-off-by: Benjamin Poirier <bpoirier@suse.com>

Thanks for sending this. I'll apply it. Should this be marked for
stable? And if so, how far back?

-- Steve

> ---
>  scripts/kconfig/streamline_config.pl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> index f3d3fb4..7036ae3 100755
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -188,7 +188,7 @@ sub read_kconfig {
>  	$cont = 0;
>  
>  	# collect any Kconfig sources
> -	if (/^source\s*"(.*)"/) {
> +	if (/^source\s+"?([^"]+)/) {
>  	    my $kconfig = $1;
>  	    # prevent reading twice.
>  	    if (!defined($read_kconfigs{$kconfig})) {

Re: [PATCH 1/2] localmodconfig: Fix parsing of Kconfig "source" statements

[Benjamin Poirier]

On 2016/04/08 14:29, Steven Rostedt wrote:
> On Sat,  2 Apr 2016 10:55:21 -0700
> Benjamin Poirier <bpoirier@suse.com> wrote:
> 
> > The parameter of Kconfig "source" statements does not need to be quoted.
> > The current regex causes many kconfig files to be skipped and hence,
> > dependencies to be missed.
> > 
> > Also fix the whitespace repeat count.
> > 
> > Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
> 
> Thanks for sending this. I'll apply it. Should this be marked for
> stable? And if so, how far back?

The first problem dates back to the introduction of streamline_config.pl in
	dcc6024 kconfig: add streamline_config.pl to scripts (v2.6.32-rc1)
The second problem started with
	19e91b6 modsign: Allow external signing key to be specified (v4.3-rc1)

However, I'm not sure that adding the patch to stable is warranted. I
considered this with regards to the problem fixed by patch 1/2.

First, I searched for cases where dependency info that's currently missing
would lead to lead to symbols being erroneously deactivated but I did not find
any. Since these cases are not so obvious and are quite rare in general, it's
possible that I missed one but at this stage the problem is theoretical.

Second, even if there is such a case, I'm not sure that the problem is
"critical". streamline_config.pl may output an "invalid" config because
it misses some dependencies but the config will be fixed by the
invocation of silentoldconfig that comes right after in the
localmodconfig makefile rule. It might not be the config the user
wanted, but it will be valid.

However, while looking for such a case, I've noticed a few other issues in
streamline_config.pl and I'll send more patches shortly.

> 
> -- Steve
> 
> > ---
> >  scripts/kconfig/streamline_config.pl | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> > index f3d3fb4..7036ae3 100755
> > --- a/scripts/kconfig/streamline_config.pl
> > +++ b/scripts/kconfig/streamline_config.pl
> > @@ -188,7 +188,7 @@ sub read_kconfig {
> >  	$cont = 0;
> >  
> >  	# collect any Kconfig sources
> > -	if (/^source\s*"(.*)"/) {
> > +	if (/^source\s+"?([^"]+)/) {
> >  	    my $kconfig = $1;
> >  	    # prevent reading twice.
> >  	    if (!defined($read_kconfigs{$kconfig})) {
> 
> 

[PATCH 1/4] localmodconfig: Recognize more keywords that end a menu entry

[Benjamin Poirier]

Based on the list in Documentation/kbuild/kconfig-language.txt

This removes junk from %depends because parsing of a menu entry spilled
over to another menu entry.

Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
---
 scripts/kconfig/streamline_config.pl | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index 514735d..c5b388c 100755
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -256,8 +256,8 @@ sub read_kconfig {
 
 	    $iflevel-- if ($iflevel);
 
-	# stop on "help"
-	} elsif (/^\s*help\s*$/) {
+	# stop on "help" and keywords that end a menu entry
+	} elsif (/^\s*help\s*$/ || /^(comment|choice|menu)\b/) {
 	    $state = "NONE";
 	}
     }
-- 
2.7.4

[PATCH 2/4] localmodconfig: Fix parsing of "help" text

[Benjamin Poirier]

Help text may start with "help" or "---help---". This patch fixes
read_kconfig() to recognize the second variant.

This removes useless junk from %depends and %selects. That junk is due to
help text that contains the words "selects" and "depends".

Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
---
 scripts/kconfig/streamline_config.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index c5b388c..f06972a 100755
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -257,7 +257,7 @@ sub read_kconfig {
 	    $iflevel-- if ($iflevel);
 
 	# stop on "help" and keywords that end a menu entry
-	} elsif (/^\s*help\s*$/ || /^(comment|choice|menu)\b/) {
+	} elsif (/^\s*(---)?help(---)?\s*$/ || /^(comment|choice|menu)\b/) {
 	    $state = "NONE";
 	}
     }
-- 
2.7.4

[PATCH 3/4] localmodconfig: Add missing $ to reference a variable

[Benjamin Poirier]

That is clearly what the original intention was. This does not change the
output .config but it prevents some useless processing.

! eq "m" is changed to the simpler eq "y"; symbols with values other than
m|y are not included in %orig_configs.

Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
---
 scripts/kconfig/streamline_config.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index f06972a..bbc160c 100755
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -454,7 +454,7 @@ sub parse_config_depends
 	    $p =~ s/^[^$valid]*[$valid]+//;
 
 	    # We only need to process if the depend config is a module
-	    if (!defined($orig_configs{$conf}) || !$orig_configs{conf} eq "m") {
+	    if (!defined($orig_configs{$conf}) || $orig_configs{$conf} eq "y") {
 		next;
 	    }
 
-- 
2.7.4

[PATCH 4/4] localmodconfig: Recognize standalone "prompt"

[Benjamin Poirier]

Note that this may change the resulting .config, causing it to have fewer
symbols turned on. Before this patch we incorrectly identified some symbols
as not having a prompt and needing to be selected by something else.

Also fix the whitespace repeat after "tristate".

Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
---
 scripts/kconfig/streamline_config.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index bbc160c..d465672 100755
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -237,7 +237,7 @@ sub read_kconfig {
 	    }
 
 	# configs without prompts must be selected
-	} elsif ($state ne "NONE" && /^\s*tristate\s\S/) {
+	} elsif ($state ne "NONE" && /^\s*(tristate\s+\S|prompt\b)/) {
 	    # note if the config has a prompt
 	    $prompts{$config} = 1;
 
-- 
2.7.4

Re: [PATCH 4/4] localmodconfig: Recognize standalone "prompt"

[Steven Rostedt]

On Sun, 10 Apr 2016 17:06:33 -0700
Benjamin Poirier <bpoirier@suse.com> wrote:

> Note that this may change the resulting .config, causing it to have fewer
> symbols turned on. Before this patch we incorrectly identified some symbols
> as not having a prompt and needing to be selected by something else.
> 
> Also fix the whitespace repeat after "tristate".
> 
> Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
> ---
>  scripts/kconfig/streamline_config.pl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> index bbc160c..d465672 100755
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -237,7 +237,7 @@ sub read_kconfig {
>  	    }
>  
>  	# configs without prompts must be selected
> -	} elsif ($state ne "NONE" && /^\s*tristate\s\S/) {
> +	} elsif ($state ne "NONE" && /^\s*(tristate\s+\S|prompt\b)/) {

I'm curious. What modules have tristate and a specified prompt?

That is, what modules are removed with this patch?

I applied the first three so far.

Thanks,

-- Steve

>  	    # note if the config has a prompt
>  	    $prompts{$config} = 1;
>  

Re: [PATCH 4/4] localmodconfig: Recognize standalone "prompt"

[Benjamin Poirier]

On 2016/04/26 11:11, Steven Rostedt wrote:
> On Sun, 10 Apr 2016 17:06:33 -0700
> Benjamin Poirier <bpoirier@suse.com> wrote:
> 
> > Note that this may change the resulting .config, causing it to have fewer
> > symbols turned on. Before this patch we incorrectly identified some symbols
> > as not having a prompt and needing to be selected by something else.
> > 
> > Also fix the whitespace repeat after "tristate".
> > 
> > Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
> > ---
> >  scripts/kconfig/streamline_config.pl | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> > index bbc160c..d465672 100755
> > --- a/scripts/kconfig/streamline_config.pl
> > +++ b/scripts/kconfig/streamline_config.pl
> > @@ -237,7 +237,7 @@ sub read_kconfig {
> >  	    }
> >  
> >  	# configs without prompts must be selected
> > -	} elsif ($state ne "NONE" && /^\s*tristate\s\S/) {
> > +	} elsif ($state ne "NONE" && /^\s*(tristate\s+\S|prompt\b)/) {
> 
> I'm curious. What modules have tristate and a specified prompt?

Actually, I didn't find config symbols that are tristate and have a separate
prompt. However, the following tristate symbols were not recognized by the
original expression because they are defined with two spaces between
"tristate" and the prompt string (sigh):
	IP6_NF_RAW
	IP_NF_RAW
	NETFILTER_XT_MATCH_COMMENT
	NETFILTER_XT_MATCH_CONNBYTES
	NETFILTER_XT_MATCH_CONNMARK
	NETFILTER_XT_MATCH_REALM
	NETFILTER_XT_MATCH_SCTP
	NETFILTER_XT_MATCH_STRING
	NETFILTER_XT_TARGET_CONNMARK
	NETFILTER_XT_TARGET_IDLETIMER
	NETFILTER_XT_TARGET_NOTRACK
	NETFILTER_XT_TARGET_TRACE
	NF_CT_NETLINK_TIMEOUT
	USB_EHCI_HCD_AT91
	USB_EHCI_HCD_ORION

So they were not added to %prompts and now they are.

> 
> That is, what modules are removed with this patch?
> 

None, it turns out. Because it is the case that none of the above symbols are
selected by other symbols; the second condition in
	    if (!defined($prompts{$config}) && defined($selects{$config})) {
		$process_selects{$config} = 1;
	    }
is always false for these symbols.

I've updated the patch subject and log accordingly.

[PATCH v2] localmodconfig: Fix whitespace repeat count after "tristate"

[Benjamin Poirier]

Also recognize standalone "prompt".

Before this patch we incorrectly identified some symbols as not having a
prompt and potentially needing to be selected by something else.

Note that this patch could theoretically change the resulting .config,
causing it to have fewer symbols turned on. However, given the current set
of Kconfig files, this situation does not occur because the symbols newly
added to %prompts are absent from %selects.

Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
---
 scripts/kconfig/streamline_config.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index 95a6f2b..b8c7b29 100755
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -237,7 +237,7 @@ sub read_kconfig {
 	    }
 
 	# configs without prompts must be selected
-	} elsif ($state ne "NONE" && /^\s*tristate\s\S/) {
+	} elsif ($state ne "NONE" && /^\s*(tristate\s+\S|prompt\b)/) {
 	    # note if the config has a prompt
 	    $prompts{$config} = 1;
 
-- 
2.7.4

Re: [PATCH v2] localmodconfig: Fix whitespace repeat count after "tristate"

[Steven Rostedt]

On Tue, 26 Apr 2016 11:56:38 -0700
Benjamin Poirier <bpoirier@suse.com> wrote:

> Also recognize standalone "prompt".
> 
> Before this patch we incorrectly identified some symbols as not having a
> prompt and potentially needing to be selected by something else.
> 
> Note that this patch could theoretically change the resulting .config,
> causing it to have fewer symbols turned on. However, given the current set
> of Kconfig files, this situation does not occur because the symbols newly
> added to %prompts are absent from %selects.
> 
> Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
> ---
>  scripts/kconfig/streamline_config.pl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> index 95a6f2b..b8c7b29 100755
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -237,7 +237,7 @@ sub read_kconfig {
>  	    }
>  
>  	# configs without prompts must be selected
> -	} elsif ($state ne "NONE" && /^\s*tristate\s\S/) {
> +	} elsif ($state ne "NONE" && /^\s*(tristate\s+\S|prompt\b)/) {

I prefer not to have the "prompt" here. I'm only interested in module
configs. But the \s to \s+ is a real fix.

Can you resend?

-- Steve

>  	    # note if the config has a prompt
>  	    $prompts{$config} = 1;
>  

Re: [PATCH v2] localmodconfig: Fix whitespace repeat count after "tristate"

[Benjamin Poirier]

On 2016/04/26 15:51, Steven Rostedt wrote:
> On Tue, 26 Apr 2016 11:56:38 -0700
> Benjamin Poirier <bpoirier@suse.com> wrote:
> 
> > Also recognize standalone "prompt".
> > 
> > Before this patch we incorrectly identified some symbols as not having a
> > prompt and potentially needing to be selected by something else.
> > 
> > Note that this patch could theoretically change the resulting .config,
> > causing it to have fewer symbols turned on. However, given the current set
> > of Kconfig files, this situation does not occur because the symbols newly
> > added to %prompts are absent from %selects.
> > 
> > Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
> > ---
> >  scripts/kconfig/streamline_config.pl | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> > index 95a6f2b..b8c7b29 100755
> > --- a/scripts/kconfig/streamline_config.pl
> > +++ b/scripts/kconfig/streamline_config.pl
> > @@ -237,7 +237,7 @@ sub read_kconfig {
> >  	    }
> >  
> >  	# configs without prompts must be selected
> > -	} elsif ($state ne "NONE" && /^\s*tristate\s\S/) {
> > +	} elsif ($state ne "NONE" && /^\s*(tristate\s+\S|prompt\b)/) {
> 
> I prefer not to have the "prompt" here. I'm only interested in module
> configs. But the \s to \s+ is a real fix.

Separate tristate and prompt statements are allowed and recognized by
the kconfig parser. Although there are no such cases now, there used to
be one: 0a57274
You sure you want to remove it from the regexp?

Re: [PATCH v2] localmodconfig: Fix whitespace repeat count after "tristate"

[Steven Rostedt]

On Tue, 26 Apr 2016 14:35:19 -0700
Benjamin Poirier <bpoirier@suse.com> wrote:

> Separate tristate and prompt statements are allowed and recognized by
> the kconfig parser. Although there are no such cases now, there used to
> be one: 0a57274
> You sure you want to remove it from the regexp?

Hmm, OK, I'll take your patch as is.

Thanks,

-- Steve

Re: [PATCH 1/2] localmodconfig: Fix parsing of Kconfig "source" statements

[Benjamin Poirier]

On 2016/04/10 16:52, Benjamin Poirier wrote:
> On 2016/04/08 14:29, Steven Rostedt wrote:
> > On Sat,  2 Apr 2016 10:55:21 -0700
> > Benjamin Poirier <bpoirier@suse.com> wrote:
> > 
> > > The parameter of Kconfig "source" statements does not need to be quoted.
> > > The current regex causes many kconfig files to be skipped and hence,
> > > dependencies to be missed.
> > > 
> > > Also fix the whitespace repeat count.
> > > 
> > > Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
> > 
> > Thanks for sending this. I'll apply it. Should this be marked for
> > stable? And if so, how far back?
> 
> The first problem dates back to the introduction of streamline_config.pl in
> 	dcc6024 kconfig: add streamline_config.pl to scripts (v2.6.32-rc1)
> The second problem started with
> 	19e91b6 modsign: Allow external signing key to be specified (v4.3-rc1)
> 
> However, I'm not sure that adding the patch to stable is warranted. I
> considered this with regards to the problem fixed by patch 1/2.
> 
> First, I searched for cases where dependency info that's currently missing
> would lead to lead to symbols being erroneously deactivated but I did not find
> any. Since these cases are not so obvious and are quite rare in general, it's
> possible that I missed one but at this stage the problem is theoretical.
> 
> Second, even if there is such a case, I'm not sure that the problem is
> "critical". streamline_config.pl may output an "invalid" config because
> it misses some dependencies but the config will be fixed by the
> invocation of silentoldconfig that comes right after in the
> localmodconfig makefile rule. It might not be the config the user
> wanted, but it will be valid.
> 
> However, while looking for such a case, I've noticed a few other issues in
> streamline_config.pl and I'll send more patches shortly.

Hi Steven,

Can you please merge these six patches (2 original + 4 followups)?

Thank you,
-Benjamin

> 
> > 
> > -- Steve
> > 
> > > ---
> > >  scripts/kconfig/streamline_config.pl | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> > > index f3d3fb4..7036ae3 100755
> > > --- a/scripts/kconfig/streamline_config.pl
> > > +++ b/scripts/kconfig/streamline_config.pl
> > > @@ -188,7 +188,7 @@ sub read_kconfig {
> > >  	$cont = 0;
> > >  
> > >  	# collect any Kconfig sources
> > > -	if (/^source\s*"(.*)"/) {
> > > +	if (/^source\s+"?([^"]+)/) {
> > >  	    my $kconfig = $1;
> > >  	    # prevent reading twice.
> > >  	    if (!defined($read_kconfigs{$kconfig})) {
> > 
> > 

Re: [PATCH 1/2] localmodconfig: Fix parsing of Kconfig "source" statements

[Steven Rostedt]

On Mon, 18 Apr 2016 13:29:12 -0700
Benjamin Poirier <bpoirier@suse.com> wrote:

> On 2016/04/10 16:52, Benjamin Poirier wrote:
> > On 2016/04/08 14:29, Steven Rostedt wrote:  
> > > On Sat,  2 Apr 2016 10:55:21 -0700
> > > Benjamin Poirier <bpoirier@suse.com> wrote:
> > >   
> > > > The parameter of Kconfig "source" statements does not need to be quoted.
> > > > The current regex causes many kconfig files to be skipped and hence,
> > > > dependencies to be missed.
> > > > 
> > > > Also fix the whitespace repeat count.
> > > > 
> > > > Signed-off-by: Benjamin Poirier <bpoirier@suse.com>  
> > > 
> > > Thanks for sending this. I'll apply it. Should this be marked for
> > > stable? And if so, how far back?  
> > 
> > The first problem dates back to the introduction of streamline_config.pl in
> > 	dcc6024 kconfig: add streamline_config.pl to scripts (v2.6.32-rc1)
> > The second problem started with
> > 	19e91b6 modsign: Allow external signing key to be specified (v4.3-rc1)
> > 
> > However, I'm not sure that adding the patch to stable is warranted. I
> > considered this with regards to the problem fixed by patch 1/2.
> > 
> > First, I searched for cases where dependency info that's currently missing
> > would lead to lead to symbols being erroneously deactivated but I did not find
> > any. Since these cases are not so obvious and are quite rare in general, it's
> > possible that I missed one but at this stage the problem is theoretical.
> > 
> > Second, even if there is such a case, I'm not sure that the problem is
> > "critical". streamline_config.pl may output an "invalid" config because
> > it misses some dependencies but the config will be fixed by the
> > invocation of silentoldconfig that comes right after in the
> > localmodconfig makefile rule. It might not be the config the user
> > wanted, but it will be valid.
> > 
> > However, while looking for such a case, I've noticed a few other issues in
> > streamline_config.pl and I'll send more patches shortly.  
> 
> Hi Steven,
> 
> Can you please merge these six patches (2 original + 4 followups)?
> 

OK, I've marked them as todo. I've just come back from two weeks of
conferences and I'm working on catching up.

-- Steve

Re: [PATCH 1/2] localmodconfig: Fix parsing of Kconfig "source" statements

[Benjamin Poirier]

On 2016/04/18 17:43, Steven Rostedt wrote:
[...]
> > Hi Steven,
> > 
> > Can you please merge these six patches (2 original + 4 followups)?
> > 
> 
> OK, I've marked them as todo. I've just come back from two weeks of
> conferences and I'm working on catching up.
> 

Ah I see, thanks. I just wanted to be sure they didn't fall through the
cracks.