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:0.026 / U:0.644 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site