lkml.org 
[lkml]   [2012]   [Jan]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 2/2] kconfig/streamline-config.pl: Fix parsing Makefile with variables
From
Date
Steven,

A few remarks follow. (Mainly suggesting to add a some comments.)

On Fri, 2012-01-13 at 18:12 -0500, Steven Rostedt wrote:
> From: Steven Rostedt <srostedt@redhat.com>
>
> Thomas Lange reported that when he did a 'make localmodconfig', his
> config was missing the brcmsmac driver, even though he had the module
> loaded.
>
> Looking into this, I found the file:
> drivers/net/wireless/brcm80211/brcmsmac/Makefile
> had the following in the Makefile:
>
> MODULEPFX := brcmsmac
>
> obj-$(CONFIG_BRCMSMAC) += $(MODULEPFX).o
>
> The way streamline-config.pl works, is parsing all the
> obj-$(CONFIG_FOO) += foo.o
> lines to find that CONFIG_FOO belongs to the module foo.ko.
>
> But in this case, the brcmsmac.o was not used, but a variable in its place.
>
> By changing streamline-config.pl to remember defined variables in Makefiles
> and substituting them when they are used in the obj-X lines, allows
> Thomas (and others) to have their brcmsmac module stay configured
> when it is loaded and running "make localmodconfig".
>
> Reported-by: Thomas Lange <thomas-lange2@gmx.de>
> Tested-by: Thomas Lange <thomas-lange2@gmx.de>
> Cc: Arend van Spriel <arend@broadcom.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
> scripts/kconfig/streamline_config.pl | 29 +++++++++++++++++++++++++++++
> 1 files changed, 29 insertions(+), 0 deletions(-)
>
> diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> index 42ef5ea..bccf07d 100644
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -250,10 +250,33 @@ if ($kconfig) {
> read_kconfig($kconfig);
> }
>
> +sub convert_vars {
> + my ($line, %vars) = @_;
> +
> + my $process = "";
> +
> + while ($line =~ s/^(.*?)(\$\((.*?)\))//) {

I know just enough perl to be dangerous. But I'm guessing that even
people more familiar with perl (and regexes) wouldn't mind a short
comment here. Perhaps something like
# handle all occurrences of "$(VAR)" in this line

> + my $start = $1;
> + my $variable = $2;
> + my $var = $3;

Using both "$variable" and "$var" is a bit awkward. Perhaps "$name"
instead of "$var"?

> +
> + if (defined($vars{$var})) {
> + $process .= $start . $vars{$var};
> + } else {

Add
# Unknown variable, just put it back

here (or something similar)?

> + $process .= $start . $variable;
> + }
> + }
> +
> + $process .= $line;
> +
> + return $process;
> +}
> +
> # Read all Makefiles to map the configs to the objects
> foreach my $makefile (@makefiles) {
>
> my $line = "";
> + my %make_vars;
>
> open(MIN,$makefile) || die "Can't open $makefile";
> while (<MIN>) {
> @@ -270,10 +293,16 @@ foreach my $makefile (@makefiles) {
>
> my $objs;
>
> + $_ = convert_vars($_, %make_vars);
> +
> # collect objects after obj-$(CONFIG_FOO_BAR)
> if (/obj-\$\((CONFIG_[^\)]*)\)\s*[+:]?=\s*(.*)/) {

This is unrelated to this patch, but anyhow. Doing
git grep -n "obj-" $(git ls-files "*Makefile*") | grep CONFIG | grep -v "obj-\$(CONFIG"

generated three types of possible corner cases here. Some examples:
0) "obj-pci-$(CONFIG_ARCH_IXDP4XX) += ixdp425-pci.o"
(there are about a dozen of these)
and
"obj-pvrusb2-sysfs-$(CONFIG_VIDEO_PVRUSB2_SYSFS) := pvrusb2-sysfs.o"
(three times)
Are these relevant for this script?

1) "obj-${CONFIG_EISA_PCI_EISA} += pci_eisa.o"
(three times)

The braces look a bit odd. Shouldn't that be parentheses, at least to be
consistent? Michal?

2) "obj-$(subst m,y,$(CONFIG_VLAN_8021Q)) += vlan_core.o"
(about a dozen times)
I do not know what subst actually does here, but this might be missed by
this script too. Relevant?

Do these corner cases merit further attention?

> $var = $1;
> $objs = $2;
> +
> + # check if variables are set

"[...] and safe them for future substitution"?

> + } elsif (/^\s*(\S+)\s*[:]?=\s*(.*\S)/) {
> + $make_vars{$1} = $2;
> }
> if (defined($objs)) {
> foreach my $obj (split /\s+/,$objs) {


Paul Bolle



\
 
 \ /
  Last update: 2012-01-14 07:25    [W:1.033 / U:1.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site