Patch for xconfig

Patch for xconfig

[Pete Zaitcev]

My customers complain that using certain canned configurations
xconfig does not work (naturally, it works with defconfig).
A problem that I am trying to fix is that it can refuse to
quit with something like "Variable CONSTANT_M does not exist".
The necessary "global" is indeed missing.

Can someone knowledgeable (like Chastain) have a look at
the attached patch?

Thanks,
-- Pete

--- linux-2.4.18-7.80/scripts/tkgen.c	Fri Jul 26 11:56:29 2002
+++ linux-2.4.18-7.80-xcf/scripts/tkgen.c	Fri Jul 26 13:30:45 2002
@@ -625,6 +625,7 @@
 		if ( ! vartable[i].global_written )
 		{
 		    global( vartable[i].name );
+                    vartable[i].global_written = 1;
 		}
 		printf( "\t" );
 	    }
@@ -698,6 +699,19 @@
 	}
     }
 
+    /*
+     * Generate global declarations for the dependency chain (e.g. CONSTANT_M).
+     */
+    for ( tmp = cfg->depend; tmp; tmp = tmp->next )
+    {
+        int i = get_varnum( tmp->name );
+	if ( ! vartable[i].global_written )
+	{
+	    global( vartable[i].name );
+	    vartable[i].global_written = 1;
+	}
+    }
+
     /*
      * Generate indentation.
      */

Re: Patch for xconfig

[Greg Banks]

G'day,

Pete Zaitcev wrote:
> 
> My customers complain that using certain canned configurations
> xconfig does not work (naturally, it works with defconfig).
> A problem that I am trying to fix is that it can refuse to
> quit with something like "Variable CONSTANT_M does not exist".
> The necessary "global" is indeed missing.
>
> Can someone knowledgeable (like Chastain) have a look at
> the attached patch?

I don't claim to be knowledgeable, but I can confirm that this is a
real bug and the patch fixes it.  Here is the patch re-jigged to apply
cleanly to 2.5.29.

diff -ruN --exclude-from=dontdiff linux-2.5.29-orig/scripts/tkgen.c linux-2.5.29/scripts/tkgen.c
--- linux-2.5.29-orig/scripts/tkgen.c	Sun Jul 28 22:34:05 2002
+++ linux-2.5.29/scripts/tkgen.c	Sun Jul 28 22:32:23 2002
@@ -625,6 +625,7 @@
 		if ( ! vartable[i].global_written )
 		{
 		    global( vartable[i].name );
+		    vartable[i].global_written = 1;
 		}
 		printf( "\t" );
 	    }
@@ -696,6 +697,19 @@
 	    }
 	    break;
 	}
+    }
+
+    /*
+     * Generate global declarations for the dependency chain (e.g. CONSTANT_M).
+     */
+    for ( tmp = cfg->depend; tmp; tmp = tmp->next )
+    {
+       int i = get_varnum( tmp->name );
+       if ( ! vartable[i].global_written )
+       {
+           global( vartable[i].name );
+           vartable[i].global_written = 1;
+       }
     }
 
     /*


Greg.
-- 
the price of civilisation today is a courageous willingness to prevail,
with force, if necessary, against whatever vicious and uncomprehending
enemies try to strike it down.     - Roger Sandall, The Age, 28Sep2001.

Re: Patch for xconfig

[Pete Zaitcev]

> Date: Sun, 28 Jul 2002 22:40:03 +1000
> From: Greg Banks <gnb@alphalink.com.au>

> I don't claim to be knowledgeable, but I can confirm that this is a
> real bug and the patch fixes it.  Here is the patch re-jigged to apply
> cleanly to 2.5.29.
>[...]

BTW, what I sent was a low hanged fruit that I picked.
The main bug is worse, and I have no idea how to fix it.
This is what we have in configuration:

tristate 'ISO ...' CONFIG_ISO9660_FS
dep_bool ' Tranparent ...' CONFIG_ZISOFS $CONFIG_ISO9660_FS
if [ "$CONFIG_ZISOFS" = "y" ]; then
   define_tristate CONFIG_ZISOFS_FS $CONFIG_ISO9660_FS
else
   define_tristate CONFIG_ZISOFS_FS n
fi

if [ "$CONFIG_CRAMFS" = "y" -o \
     "$CONFIG_PPP_DEFLATE" = "y" -o \
     "$CONFIG_JFFS2_FS" = "y" -o \
     "$CONFIG_ZISOFS_FS" = "y" ]; then
   define_tristate CONFIG_ZLIB_INFLATE y
else
  if [ "$CONFIG_CRAMFS" = "m" -o \
       "$CONFIG_PPP_DEFLATE" = "m" -o \
       "$CONFIG_JFFS2_FS" = "m" -o \
       "$CONFIG_ZISOFS_FS" = "m" ]; then
     define_tristate CONFIG_ZLIB_INFLATE m
  else
     tristate 'zlib decompression support' CONFIG_ZLIB_INFLATE
  fi
fi

As far as I can tell, tkgen.c does an acceptable job on the
second part; though it refuses to generate "else" and uses
de Morgan transformation instead. However, it seems that tkparse
chokes on the very innocently looking first part. The result
is that xconfig insist on zlib to be a module when it should
be compiled into the kernel; it all ends with undefined symbols.
Naturally, "make oldconfig" works correctly.

The code in the menu part of kconfig.tk fixes the problem.
In other words, the bug is only visible if someone does "make xconfig",
loads a canned configuration which we ship, then does "save
and exit" immediately. If he visits any menus, everything is ok.

-- Pete

Re: Patch for xconfig

[Andrzej Krzysztofowicz]

> BTW, what I sent was a low hanged fruit that I picked.
> The main bug is worse, and I have no idea how to fix it.
> This is what we have in configuration:
> 
> tristate 'ISO ...' CONFIG_ISO9660_FS
> dep_bool ' Tranparent ...' CONFIG_ZISOFS $CONFIG_ISO9660_FS
> if [ "$CONFIG_ZISOFS" = "y" ]; then
>    define_tristate CONFIG_ZISOFS_FS $CONFIG_ISO9660_FS
> else
>    define_tristate CONFIG_ZISOFS_FS n
> fi
> 
> if [ "$CONFIG_CRAMFS" = "y" -o \
>      "$CONFIG_PPP_DEFLATE" = "y" -o \
>      "$CONFIG_JFFS2_FS" = "y" -o \
>      "$CONFIG_ZISOFS_FS" = "y" ]; then
>    define_tristate CONFIG_ZLIB_INFLATE y
> else
>   if [ "$CONFIG_CRAMFS" = "m" -o \
>        "$CONFIG_PPP_DEFLATE" = "m" -o \
>        "$CONFIG_JFFS2_FS" = "m" -o \
>        "$CONFIG_ZISOFS_FS" = "m" ]; then
>      define_tristate CONFIG_ZLIB_INFLATE m
>   else
>      tristate 'zlib decompression support' CONFIG_ZLIB_INFLATE
>   fi
> fi

IMO, the simpliest fix is to make change like replacing:
      tristate 'zlib decompression support' CONFIG_ZLIB_INFLATE
with:
      tristate 'zlib decompression support' CONFIG_ZLIB_INFLATE_X
      define_tristate CONFIG_ZLIB_INFLATE $CONFIG_ZLIB_INFLATE_X
+ rename the option in a help file (if it exist)

(not tested, but should work)

> As far as I can tell, tkgen.c does an acceptable job on the
> second part; though it refuses to generate "else" and uses
> de Morgan transformation instead. However, it seems that tkparse
> chokes on the very innocently looking first part. The result
> is that xconfig insist on zlib to be a module when it should
> be compiled into the kernel; it all ends with undefined symbols.
> Naturally, "make oldconfig" works correctly.

No. The problem is that variales associated with eg. "tristate" clauses
are always modified (even if its condition is false) during configuration 
refreshment - to store the previous value for the option.
Not good, but it is just bad project. Nobody wanted to rewrite xconfig
as CML2 was assumed to come soon...

> The code in the menu part of kconfig.tk fixes the problem.
> In other words, the bug is only visible if someone does "make xconfig",
> loads a canned configuration which we ship, then does "save
> and exit" immediately. If he visits any menus, everything is ok.

I'm not sure it is so simple...

-- 
=======================================================================
  Andrzej M. Krzysztofowicz               ankry@mif.pg.gda.pl
  phone (48)(58) 347 14 61
Faculty of Applied Phys. & Math.,   Gdansk University of Technology

Re: Patch for xconfig

[Greg Banks]

G'day,

sorry about the delay, I had to deal with a major drama in the day job.

Pete Zaitcev wrote:
> 
> > Date: Sun, 28 Jul 2002 22:40:03 +1000
> > From: Greg Banks <gnb@alphalink.com.au>
> 
> BTW, what I sent was a low hanged fruit that I picked.
> The main bug is worse, and I have no idea how to fix it.
> This is what we have in configuration:
> 
> tristate 'ISO ...' CONFIG_ISO9660_FS
> dep_bool ' Tranparent ...' CONFIG_ZISOFS $CONFIG_ISO9660_FS
> if [ "$CONFIG_ZISOFS" = "y" ]; then
>    define_tristate CONFIG_ZISOFS_FS $CONFIG_ISO9660_FS
> else
>    define_tristate CONFIG_ZISOFS_FS n
> fi
> 
> [...]it seems that tkparse
> chokes on the very innocently looking first part. 

Actually, it's not innocent at all, the documentation does not allow
the construction "define_tristate CONFIG_FOO $CONFIG_BAR".  The last
argument has to be a tristate literal, one of "y" "m" or "n".  It might
look ok but that's because you're thinking shell not config language.

Remember, config language is *not* shell.

The relevant section from Documentation/kbuild/config-language.txt is:

> === define_tristate /symbol/ /word/
> 
> This verb assigns the value of /word/ to /symbol/.  Legal input values
> are "n", "m", and "y".

If you're trying to do what I think you're trying to do, the correct
construct is this:

dep_mbool ' Tranparent ...' CONFIG_ZISOFS $CONFIG_ISO9660_FS
if [ "$CONFIG_ZISOFS" = "y" ]; then
    if [ "$CONFIG_ISO9660_FS" = "y" ]; then
	define_tristate CONFIG_ZISOFS_FS y
    else
	define_tristate CONFIG_ZISOFS_FS m
    fi
else
   define_tristate CONFIG_ZISOFS_FS n
fi

If it's any consolation, you're not the only one to get it wrong.

arch/m68k/config.in:498:   define_tristate CONFIG_SERIAL $CONFIG_DN_SERIAL
drivers/isdn/capi/Config.in:11:      define_tristate CONFIG_ISDN_CAPI_CAPIFS $CONFIG_ISDN_CAPI_CAPI20
drivers/parport/Config.in:18:         define_tristate CONFIG_PARPORT_PC_CML1 $CONFIG_PARPORT_PC
fs/Config.in:133:   define_tristate CONFIG_EXPORTFS $CONFIG_NFSD
fs/Config.in:158:   define_tristate CONFIG_ZISOFS_FS $CONFIG_ISO9660_FS
net/ipv4/netfilter/Config.in:58:          define_tristate CONFIG_IP_NF_NAT_IRC $CONFIG_IP_NF_NAT
net/ipv4/netfilter/Config.in:67:          define_tristate CONFIG_IP_NF_NAT_FTP $CONFIG_IP_NF_NAT

> The code in the menu part of kconfig.tk fixes the problem.
> In other words, the bug is only visible if someone does "make xconfig",
> loads a canned configuration which we ship, then does "save
> and exit" immediately. If he visits any menus, everything is ok.

Ah.  I had reproduced your other problem differently, with this:

mainmenu_option next_comment
comment 'xconfig needs this menu'

tristate 'Set this symbol to ON' CONFIG_FOO

dep_tristate 'this is a dep_tristate' CONFIG_BAR $CONFIG_FOO m

endmenu

Open the menu, set FOO=m, save, kaboom.  Your patch fixes that.



-- 
the price of civilisation today is a courageous willingness to prevail,
with force, if necessary, against whatever vicious and uncomprehending
enemies try to strike it down.     - Roger Sandall, The Age, 28Sep2001.