[PATCH 1/4] kbuild: don't assign `stdout' and `stderr'

[PATCH 1/4] kbuild: don't assign `stdout' and `stderr'

[Arnaud Lacombe]

These may not be modifiable, as per C99 comment:

section 7.19.5.4, note 232:
"
The primary use of the freopen function is to change the file associated with a
standard text stream (stderr, stdin, or stdout), as those identifiers need not
be modifiable lvalues to which the value returned by the fopen function may be
assigned.
"

Use a duplicate of stdout as ncurses' output terminal, then redirect it
to /dev/null for conf_write().

This fixes nconf.c build on NetBSD.

Signed-off-by: Arnaud Lacombe <lacombar@gmail.com>
CC: Nir Tzachar <nir.tzachar@gmail.com>
---
 scripts/kconfig/nconf.c |   46 +++++++++++++++++++++-------------------------
 1 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
index 2ba71bc..e08a064 100644
--- a/scripts/kconfig/nconf.c
+++ b/scripts/kconfig/nconf.c
@@ -709,25 +709,6 @@ static const char *set_config_filename(const char *config_filename)
 	return menu_backtitle;
 }
 
-/* command = 0 is supress, 1 is restore */
-static void supress_stdout(int command)
-{
-	static FILE *org_stdout;
-	static FILE *org_stderr;
-
-	if (command == 0) {
-		org_stdout = stdout;
-		org_stderr = stderr;
-		stdout = fopen("/dev/null", "a");
-		stderr = fopen("/dev/null", "a");
-	} else {
-		fclose(stdout);
-		fclose(stderr);
-		stdout = org_stdout;
-		stderr = org_stderr;
-	}
-}
-
 /* return = 0 means we are successful.
  * -1 means go on doing what you were doing
  */
@@ -753,9 +734,7 @@ static int do_exit(void)
 	/* if we got here, the user really wants to exit */
 	switch (res) {
 	case 0:
-		supress_stdout(0);
 		res = conf_write(filename);
-		supress_stdout(1);
 		if (res)
 			btn_dialog(
 				main_window,
@@ -1449,9 +1428,7 @@ static void conf_save(void)
 		case 0:
 			if (!dialog_input_result[0])
 				return;
-			supress_stdout(0);
 			res = conf_write(dialog_input_result);
-			supress_stdout(1);
 			if (!res) {
 				char buf[1024];
 				sprintf(buf, "%s %s",
@@ -1495,6 +1472,8 @@ void setup_windows(void)
 int main(int ac, char **av)
 {
 	char *mode;
+	FILE *fp;
+	int fd;
 
 	setlocale(LC_ALL, "");
 	bindtextdomain(PACKAGE, LOCALEDIR);
@@ -1509,8 +1488,25 @@ int main(int ac, char **av)
 			single_menu_mode = 1;
 	}
 
+	/* Duplicate stdout before redirecting it */
+	fd = dup(STDOUT_FILENO);
+	if (fd < 0) {
+		perror("dup");
+		return 1;
+	}
+
+	fp = fdopen(fd, "a");
+	if (fp == NULL) {
+		perror("fdopen");
+		return 1;
+	}
+
+	freopen("/dev/null", "a", stdout);
+	freopen("/dev/null", "a", stderr);
+
 	/* Initialize curses */
-	initscr();
+	newterm(NULL, fp, stdin);
+
 	/* set color theme */
 	set_colors();
 
@@ -1521,7 +1517,7 @@ int main(int ac, char **av)
 
 	if (COLS < 75 || LINES < 20) {
 		endwin();
-		printf("Your terminal should have at "
+		fprintf(fp, "Your terminal should have at "
 			"least 20 lines and 75 columns\n");
 		return 1;
 	}
-- 
1.7.2.30.gc37d7.dirty

[PATCH 2/4] kbuild: don't include `check-lxdialog' ldflags in global HOST_LOADLIBES

[Arnaud Lacombe]

On BSD systems, `check-lxdialog' would select -lcurses as the default
curses library which would conflict with -lncurses at runtime: curses'
compatible symbols are getting handled by the system's curses library while the
ncurses-specific symbols are getting handled by the ports' ncurses.

This fixes `nconf' segmentation fault on these systems.

Signed-off-by: Arnaud Lacombe <lacombar@gmail.com>
---
 scripts/kconfig/Makefile |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index de934de..7522a8b 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -146,7 +146,6 @@ check-lxdialog  := $(srctree)/$(src)/lxdialog/check-lxdialog.sh
 # Use recursively expanded variables so we do not call gcc unless
 # we really need to do so. (Do not call gcc as part of make mrproper)
 HOST_EXTRACFLAGS = $(shell $(CONFIG_SHELL) $(check-lxdialog) -ccflags)
-HOST_LOADLIBES   = $(shell $(CONFIG_SHELL) $(check-lxdialog) -ldflags $(HOSTCC))
 
 HOST_EXTRACFLAGS += -DLOCALE
 
@@ -208,7 +207,7 @@ clean-files     += config.pot linux.pot
 PHONY += $(obj)/dochecklxdialog
 $(addprefix $(obj)/,$(lxdialog)): $(obj)/dochecklxdialog
 $(obj)/dochecklxdialog:
-	$(Q)$(CONFIG_SHELL) $(check-lxdialog) -check $(HOSTCC) $(HOST_EXTRACFLAGS) $(HOST_LOADLIBES)
+	$(Q)$(CONFIG_SHELL) $(check-lxdialog) -check $(HOSTCC) $(HOST_EXTRACFLAGS) $(HOSTLOADLIBES_mconf)
 
 always := dochecklxdialog
 
@@ -226,6 +225,8 @@ HOSTLOADLIBES_gconf	= `pkg-config --libs gtk+-2.0 gmodule-2.0 libglade-2.0` -ldl
 HOSTCFLAGS_gconf.o	= `pkg-config --cflags gtk+-2.0 gmodule-2.0 libglade-2.0` \
                           -D LKC_DIRECT_LINK
 
+HOSTLOADLIBES_mconf   = $(shell $(CONFIG_SHELL) $(check-lxdialog) -ldflags $(HOSTCC))
+
 HOSTLOADLIBES_nconf	= -lmenu -lpanel -lncurses
 $(obj)/qconf.o: $(obj)/.tmp_qtcheck
 
-- 
1.7.2.30.gc37d7.dirty

[PATCH 3/4] kbuild: `nconf' needs -lintl

[Arnaud Lacombe]

Signed-off-by: Arnaud Lacombe <lacombar@gmail.com>
---
 scripts/kconfig/Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index 7522a8b..7d129f0 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -227,7 +227,7 @@ HOSTCFLAGS_gconf.o	= `pkg-config --cflags gtk+-2.0 gmodule-2.0 libglade-2.0` \
 
 HOSTLOADLIBES_mconf   = $(shell $(CONFIG_SHELL) $(check-lxdialog) -ldflags $(HOSTCC))
 
-HOSTLOADLIBES_nconf	= -lmenu -lpanel -lncurses
+HOSTLOADLIBES_nconf	= -lmenu -lpanel -lncurses -lintl
 $(obj)/qconf.o: $(obj)/.tmp_qtcheck
 
 ifeq ($(qconf-target),1)
-- 
1.7.2.30.gc37d7.dirty

[PATCH 4/4] kbuild: fix typo

[Arnaud Lacombe]

Signed-off-by: Arnaud Lacombe <lacombar@gmail.com>
---
 scripts/kconfig/nconf.gui.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
index a9d9344..b573845 100644
--- a/scripts/kconfig/nconf.gui.c
+++ b/scripts/kconfig/nconf.gui.c
@@ -137,7 +137,7 @@ void set_colors()
 	if (has_colors()) {
 		normal_color_theme();
 	} else {
-		/* give deafults */
+		/* give defaults */
 		no_colors_theme();
 	}
 }
-- 
1.7.2.30.gc37d7.dirty

Re: [PATCH 1/4] kbuild: don't assign `stdout' and `stderr'

[Michal Marek]

On Mon, Aug 16, 2010 at 12:19:03AM -0400, Arnaud Lacombe wrote:
> These may not be modifiable, as per C99 comment:
> 
> section 7.19.5.4, note 232:
> "
> The primary use of the freopen function is to change the file associated with a
> standard text stream (stderr, stdin, or stdout), as those identifiers need not
> be modifiable lvalues to which the value returned by the fopen function may be
> assigned.
> "
> 
> Use a duplicate of stdout as ncurses' output terminal, then redirect it
> to /dev/null for conf_write().

I would much rather get rid of this redirection completely. The config
library should not play with stdout, but let the frontends display
messages as they wish. This patch does it for nconfig. Nir, what do you
think?


diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index dc11d51..c07060a 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -19,6 +19,9 @@
 static void conf_warning(const char *fmt, ...)
 	__attribute__ ((format (printf, 1, 2)));
 
+static void conf_message(const char *fmt, ...)
+	__attribute__ ((format (printf, 1, 2)));
+
 static const char *conf_filename;
 static int conf_lineno, conf_warnings, conf_unsaved;
 
@@ -35,6 +38,29 @@ static void conf_warning(const char *fmt, ...)
 	conf_warnings++;
 }
 
+static void conf_default_message_callback(const char *fmt, va_list ap)
+{
+	printf("#\n# ");
+	vprintf(fmt, ap);
+	printf("\n#\n");
+}
+
+static void (*conf_message_callback) (const char *fmt, va_list ap) =
+	conf_default_message_callback;
+void conf_set_message_callback(void (*fn) (const char *fmt, va_list ap))
+{
+	conf_message_callback = fn;
+}
+
+static void conf_message(const char *fmt, ...)
+{
+	va_list ap;
+
+	va_start(ap, fmt);
+	if (conf_message_callback)
+		conf_message_callback(fmt, ap);
+}
+
 const char *conf_get_configname(void)
 {
 	char *name = getenv("KCONFIG_CONFIG");
@@ -184,9 +210,8 @@ int conf_read_simple(const char *name, int def)
 			name = conf_expand_value(prop->expr->left.sym->name);
 			in = zconf_fopen(name);
 			if (in) {
-				printf(_("#\n"
-					 "# using defaults found in %s\n"
-					 "#\n"), name);
+				conf_message(_("using defaults found in %s"),
+					 name);
 				goto load;
 			}
 		}
@@ -651,9 +676,7 @@ next:
 			return 1;
 	}
 
-	printf(_("#\n"
-		 "# configuration written to %s\n"
-		 "#\n"), newname);
+	conf_message(_("configuration written to %s"), newname);
 
 	sym_set_change_count(0);
 
diff --git a/scripts/kconfig/lkc_proto.h b/scripts/kconfig/lkc_proto.h
index 9a948c9..481d484 100644
--- a/scripts/kconfig/lkc_proto.h
+++ b/scripts/kconfig/lkc_proto.h
@@ -1,3 +1,4 @@
+#include <stdarg.h>
 
 /* confdata.c */
 P(conf_parse,void,(const char *name));
@@ -8,6 +9,7 @@ P(conf_write,int,(const char *name));
 P(conf_write_autoconf,int,(void));
 P(conf_get_changed,bool,(void));
 P(conf_set_changed_callback, void,(void (*fn)(void)));
+P(conf_set_message_callback, void,(void (*fn)(const char *fmt, va_list ap)));
 
 /* menu.c */
 P(rootmenu,struct menu,);
diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
index 18a215d..16233a9 100644
--- a/scripts/kconfig/nconf.c
+++ b/scripts/kconfig/nconf.c
@@ -651,25 +651,6 @@ static const char *set_config_filename(const char *config_filename)
 	return menu_backtitle;
 }
 
-/* command = 0 is supress, 1 is restore */
-static void supress_stdout(int command)
-{
-	static FILE *org_stdout;
-	static FILE *org_stderr;
-
-	if (command == 0) {
-		org_stdout = stdout;
-		org_stderr = stderr;
-		stdout = fopen("/dev/null", "a");
-		stderr = fopen("/dev/null", "a");
-	} else {
-		fclose(stdout);
-		fclose(stderr);
-		stdout = org_stdout;
-		stderr = org_stderr;
-	}
-}
-
 /* return = 0 means we are successful.
  * -1 means go on doing what you were doing
  */
@@ -695,9 +676,7 @@ static int do_exit(void)
 	/* if we got here, the user really wants to exit */
 	switch (res) {
 	case 0:
-		supress_stdout(0);
 		res = conf_write(filename);
-		supress_stdout(1);
 		if (res)
 			btn_dialog(
 				main_window,
@@ -707,19 +686,6 @@ static int do_exit(void)
 				  "changes were NOT saved."),
 				  1,
 				  "<OK>");
-		else {
-			char buf[1024];
-			snprintf(buf, 1024,
-				_("Configuration written to %s\n"
-				  "End of Linux kernel configuration.\n"
-				  "Execute 'make' to build the kernel or try"
-				  " 'make help'."), filename);
-			btn_dialog(
-				main_window,
-				buf,
-				1,
-				"<OK>");
-		}
 		break;
 	default:
 		btn_dialog(
@@ -1255,6 +1221,14 @@ static void conf(struct menu *menu)
 	}
 }
 
+static void conf_message_callback(const char *fmt, va_list ap)
+{
+	char buf[1024];
+
+	vsnprintf(buf, sizeof(buf), fmt, ap);
+	btn_dialog(main_window, buf, 1, "<OK>");
+}
+
 static void show_help(struct menu *menu)
 {
 	struct gstr help = str_new();
@@ -1477,16 +1451,8 @@ static void conf_save(void)
 		case 0:
 			if (!dialog_input_result[0])
 				return;
-			supress_stdout(0);
 			res = conf_write(dialog_input_result);
-			supress_stdout(1);
 			if (!res) {
-				char buf[1024];
-				sprintf(buf, "%s %s",
-					_("configuration file saved to: "),
-					dialog_input_result);
-				btn_dialog(main_window,
-					   buf, 1, "<OK>");
 				set_config_filename(dialog_input_result);
 				return;
 			}
@@ -1579,6 +1545,7 @@ int main(int ac, char **av)
 				_(menu_no_f_instructions));
 	}
 
+	conf_set_message_callback(conf_message_callback);
 	/* do the work */
 	while (!global_exit) {
 		conf(&rootmenu);

Re: [PATCH 3/4] kbuild: `nconf' needs -lintl

[Michal Marek]

On 16.8.2010 06:19, Arnaud Lacombe wrote:
> -HOSTLOADLIBES_nconf	= -lmenu -lpanel -lncurses
> +HOSTLOADLIBES_nconf	= -lmenu -lpanel -lncurses -lintl

This will not work on Linux though, the libintl functions are part of
glibc itself. There is already scripts/kconfig/check.sh and the
following snippet in scripts/kconfig/lkc.h

#ifndef KBUILD_NO_NLS
# include <libintl.h>
#else
static inline const char *gettext(const char *txt) { return txt; }
static inline void textdomain(const char *domainname) {}
static inline void bindtextdomain(const char *name, const char *dir) {}
#endif

that take care of disabling libintl support if it doesn't work. Not
optimal, but it should build in theory. Could you debug why it does not
work on NetBSD?

Michal

Re: [PATCH 2/4] kbuild: don't include `check-lxdialog' ldflags in global HOST_LOADLIBES

[Michal Marek]

On 16.8.2010 06:19, Arnaud Lacombe wrote:
> On BSD systems, `check-lxdialog' would select -lcurses as the default
> curses library which would conflict with -lncurses at runtime: curses'
> compatible symbols are getting handled by the system's curses library while the
> ncurses-specific symbols are getting handled by the ports' ncurses.
> 
> This fixes `nconf' segmentation fault on these systems.
> 
> Signed-off-by: Arnaud Lacombe <lacombar@gmail.com>
> ---
>  scripts/kconfig/Makefile |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)

I applied this and the typo fix.

Thanks,
Michal

Re: [PATCH 1/4] kbuild: don't assign `stdout' and `stderr'

[Arnaud Lacombe]

Hi,

On Tue, Aug 17, 2010 at 4:27 AM, Michal Marek <mmarek@suse.cz> wrote:
> On Mon, Aug 16, 2010 at 12:19:03AM -0400, Arnaud Lacombe wrote:
>> These may not be modifiable, as per C99 comment:
>>
>> section 7.19.5.4, note 232:
>> "
>> The primary use of the freopen function is to change the file associated with a
>> standard text stream (stderr, stdin, or stdout), as those identifiers need not
>> be modifiable lvalues to which the value returned by the fopen function may be
>> assigned.
>> "
>>
>> Use a duplicate of stdout as ncurses' output terminal, then redirect it
>> to /dev/null for conf_write().
>
> I would much rather get rid of this redirection completely. The config
> library should not play with stdout, but let the frontends display
> messages as they wish.
>
sounds reasonable to me, I didn't want at first to dig too much in
kconfig's internals, which would have lead to modification in other
part of the code.

 - Arnaud

Re: [PATCH 3/4] kbuild: `nconf' needs -lintl

[Arnaud Lacombe]

Hi,

2010/8/17 Michal Marek <mmarek@suse.cz>:
> On 16.8.2010 06:19, Arnaud Lacombe wrote:
>> -HOSTLOADLIBES_nconf  = -lmenu -lpanel -lncurses
>> +HOSTLOADLIBES_nconf  = -lmenu -lpanel -lncurses -lintl
>
> This will not work on Linux though, the libintl functions are part of
> glibc itself. There is already scripts/kconfig/check.sh and the
> following snippet in scripts/kconfig/lkc.h
>
> #ifndef KBUILD_NO_NLS
> # include <libintl.h>
> #else
> static inline const char *gettext(const char *txt) { return txt; }
> static inline void textdomain(const char *domainname) {}
> static inline void bindtextdomain(const char *name, const char *dir) {}
> #endif
>
> that take care of disabling libintl support if it doesn't work. Not
> optimal, but it should build in theory. Could you debug why it does not
> work on NetBSD?
>
there seems to be competition between setting HOST_EXTRACFLAGS from
the command line and its assignment in the Makefile:

% gmake -v
GNU Make 3.81

Using the `menuconfig' target:

works:
% gmake V=1 menuconfig
[...]
sh //linux-2.6-stable/scripts/kconfig/lxdialog/check-lxdialog.sh
-check gcc -DCURSES_LOC="<curses.h>" -DLOCALE -DKBUILD_NO_NLS -lcurses
[...]

does not work:
% gmake HOST_EXTRACFLAGS="-DUMMY=1" V=1 menuconfig
[...]
sh /linux-2.6-stable/scripts/kconfig/lxdialog/check-lxdialog.sh -check
gcc -DUMMY=1 -lcurses
[...]

The same apply with `nconfig'. It is built with the command line
HOST_EXTRACFLAGS and the one in the Makefile is ignored.

As a side note, `nconfig' on NetBSD must be build with:

HOST_EXTRACFLAGS:="-I/usr/pkg/include/ -I/usr/pkg/include/ncurses/"
HOSTLDFLAGS="-Wl,-rpath=/usr/pkg/lib/ -L/usr/pkg/lib/"

as ncurses is not the default system curses library.

 - Arnaud

Re: [PATCH 1/4] kbuild: don't assign `stdout' and `stderr'

[Nir Tzachar]

On Tue, Aug 17, 2010 at 11:27 AM, Michal Marek <mmarek@suse.cz> wrote:
> On Mon, Aug 16, 2010 at 12:19:03AM -0400, Arnaud Lacombe wrote:
>> These may not be modifiable, as per C99 comment:
>>
>> section 7.19.5.4, note 232:
>> "
>> The primary use of the freopen function is to change the file associated with a
>> standard text stream (stderr, stdin, or stdout), as those identifiers need not
>> be modifiable lvalues to which the value returned by the fopen function may be
>> assigned.
>> "
>>
>> Use a duplicate of stdout as ncurses' output terminal, then redirect it
>> to /dev/null for conf_write().
>
> I would much rather get rid of this redirection completely. The config
> library should not play with stdout, but let the frontends display
> messages as they wish. This patch does it for nconfig. Nir, what do you
> think?
>
>
> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> index dc11d51..c07060a 100644
> --- a/scripts/kconfig/confdata.c
> +++ b/scripts/kconfig/confdata.c
> @@ -19,6 +19,9 @@
>  static void conf_warning(const char *fmt, ...)
>        __attribute__ ((format (printf, 1, 2)));
>
> +static void conf_message(const char *fmt, ...)
> +       __attribute__ ((format (printf, 1, 2)));
> +
>  static const char *conf_filename;
>  static int conf_lineno, conf_warnings, conf_unsaved;
>
> @@ -35,6 +38,29 @@ static void conf_warning(const char *fmt, ...)
>        conf_warnings++;
>  }
>
> +static void conf_default_message_callback(const char *fmt, va_list ap)
> +{
> +       printf("#\n# ");
> +       vprintf(fmt, ap);
> +       printf("\n#\n");
> +}
> +
> +static void (*conf_message_callback) (const char *fmt, va_list ap) =
> +       conf_default_message_callback;
> +void conf_set_message_callback(void (*fn) (const char *fmt, va_list ap))
> +{
> +       conf_message_callback = fn;
> +}
> +
> +static void conf_message(const char *fmt, ...)
> +{
> +       va_list ap;
> +
> +       va_start(ap, fmt);
> +       if (conf_message_callback)
> +               conf_message_callback(fmt, ap);
> +}
> +
>  const char *conf_get_configname(void)
>  {
>        char *name = getenv("KCONFIG_CONFIG");
> @@ -184,9 +210,8 @@ int conf_read_simple(const char *name, int def)
>                        name = conf_expand_value(prop->expr->left.sym->name);
>                        in = zconf_fopen(name);
>                        if (in) {
> -                               printf(_("#\n"
> -                                        "# using defaults found in %s\n"
> -                                        "#\n"), name);
> +                               conf_message(_("using defaults found in %s"),
> +                                        name);
>                                goto load;
>                        }
>                }
> @@ -651,9 +676,7 @@ next:
>                        return 1;
>        }
>
> -       printf(_("#\n"
> -                "# configuration written to %s\n"
> -                "#\n"), newname);
> +       conf_message(_("configuration written to %s"), newname);
>
>        sym_set_change_count(0);
>
> diff --git a/scripts/kconfig/lkc_proto.h b/scripts/kconfig/lkc_proto.h
> index 9a948c9..481d484 100644
> --- a/scripts/kconfig/lkc_proto.h
> +++ b/scripts/kconfig/lkc_proto.h
> @@ -1,3 +1,4 @@
> +#include <stdarg.h>
>
>  /* confdata.c */
>  P(conf_parse,void,(const char *name));
> @@ -8,6 +9,7 @@ P(conf_write,int,(const char *name));
>  P(conf_write_autoconf,int,(void));
>  P(conf_get_changed,bool,(void));
>  P(conf_set_changed_callback, void,(void (*fn)(void)));
> +P(conf_set_message_callback, void,(void (*fn)(const char *fmt, va_list ap)));
>
>  /* menu.c */
>  P(rootmenu,struct menu,);
> diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
> index 18a215d..16233a9 100644
> --- a/scripts/kconfig/nconf.c
> +++ b/scripts/kconfig/nconf.c
> @@ -651,25 +651,6 @@ static const char *set_config_filename(const char *config_filename)
>        return menu_backtitle;
>  }
>
> -/* command = 0 is supress, 1 is restore */
> -static void supress_stdout(int command)
> -{
> -       static FILE *org_stdout;
> -       static FILE *org_stderr;
> -
> -       if (command == 0) {
> -               org_stdout = stdout;
> -               org_stderr = stderr;
> -               stdout = fopen("/dev/null", "a");
> -               stderr = fopen("/dev/null", "a");
> -       } else {
> -               fclose(stdout);
> -               fclose(stderr);
> -               stdout = org_stdout;
> -               stderr = org_stderr;
> -       }
> -}
> -
>  /* return = 0 means we are successful.
>  * -1 means go on doing what you were doing
>  */
> @@ -695,9 +676,7 @@ static int do_exit(void)
>        /* if we got here, the user really wants to exit */
>        switch (res) {
>        case 0:
> -               supress_stdout(0);
>                res = conf_write(filename);
> -               supress_stdout(1);
>                if (res)
>                        btn_dialog(
>                                main_window,
> @@ -707,19 +686,6 @@ static int do_exit(void)
>                                  "changes were NOT saved."),
>                                  1,
>                                  "<OK>");
> -               else {
> -                       char buf[1024];
> -                       snprintf(buf, 1024,
> -                               _("Configuration written to %s\n"
> -                                 "End of Linux kernel configuration.\n"
> -                                 "Execute 'make' to build the kernel or try"
> -                                 " 'make help'."), filename);
> -                       btn_dialog(
> -                               main_window,
> -                               buf,
> -                               1,
> -                               "<OK>");
> -               }
>                break;
>        default:
>                btn_dialog(
> @@ -1255,6 +1221,14 @@ static void conf(struct menu *menu)
>        }
>  }
>
> +static void conf_message_callback(const char *fmt, va_list ap)
> +{
> +       char buf[1024];
> +
> +       vsnprintf(buf, sizeof(buf), fmt, ap);
> +       btn_dialog(main_window, buf, 1, "<OK>");
> +}
> +
>  static void show_help(struct menu *menu)
>  {
>        struct gstr help = str_new();
> @@ -1477,16 +1451,8 @@ static void conf_save(void)
>                case 0:
>                        if (!dialog_input_result[0])
>                                return;
> -                       supress_stdout(0);
>                        res = conf_write(dialog_input_result);
> -                       supress_stdout(1);
>                        if (!res) {
> -                               char buf[1024];
> -                               sprintf(buf, "%s %s",
> -                                       _("configuration file saved to: "),
> -                                       dialog_input_result);
> -                               btn_dialog(main_window,
> -                                          buf, 1, "<OK>");
>                                set_config_filename(dialog_input_result);
>                                return;
>                        }
> @@ -1579,6 +1545,7 @@ int main(int ac, char **av)
>                                _(menu_no_f_instructions));
>        }
>
> +       conf_set_message_callback(conf_message_callback);
>        /* do the work */
>        while (!global_exit) {
>                conf(&rootmenu);
>

Hi.

This seems like the better solution. However, We should retain the
'Configuration complete. Use 'make' to compile the kernel or 'make
help' to get help" message. Probably as a printf at the end of
nconfig.

Cheers.

[PATCH] kbuild: don't overwrite HOST_EXTRACFLAGS

[Arnaud Lacombe]

These might be used by the user to specify extra arguments for the host
compiler.

Signed-off-by: Arnaud Lacombe <lacombar@gmail.com>
---
 scripts/kconfig/Makefile |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index 7d129f0..051eba2 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -145,10 +145,8 @@ check-lxdialog  := $(srctree)/$(src)/lxdialog/check-lxdialog.sh
 
 # Use recursively expanded variables so we do not call gcc unless
 # we really need to do so. (Do not call gcc as part of make mrproper)
-HOST_EXTRACFLAGS = $(shell $(CONFIG_SHELL) $(check-lxdialog) -ccflags)
-
-HOST_EXTRACFLAGS += -DLOCALE
-
+HOST_EXTRACFLAGS += $(shell $(CONFIG_SHELL) $(check-lxdialog) -ccflags) \
+                    -DLOCALE
 
 # ===========================================================================
 # Shared Makefile for the various kconfig executables:
-- 
1.7.2.30.gc37d7.dirty

Re: [PATCH] kbuild: don't overwrite HOST_EXTRACFLAGS

[Arnaud Lacombe]

Hi,

On Sun, Aug 22, 2010 at 8:03 PM, Arnaud Lacombe <lacombar@gmail.com> wrote:
> These might be used by the user to specify extra arguments for the host
> compiler.
>
Do'h... the original patch was against -rc1 and changed both
HOST_EXTRACFLAGS and HOST_LOADLIBES, but I cherry-picked it to be
against your `for-next' and more particularly commit 7080e47bb, which
removes the assignation of HOST_LOADLIBES. As I didn't pay attention
and kept the plural :/

Anyway, with this patch and specifying HOST_EXTRACFLAGS in the
environment rather than as a gmake(1) argument, it is properly
appended. nconf links and runs fine on NetBSD. It should not impact
other environment.

 - Arnaud

Re: [PATCH] kbuild: don't overwrite HOST_EXTRACFLAGS

[Michal Marek]

On Sun, Aug 22, 2010 at 08:17:21PM -0400, Arnaud Lacombe wrote:
> Hi,
> 
> On Sun, Aug 22, 2010 at 8:03 PM, Arnaud Lacombe <lacombar@gmail.com> wrote:
> > These might be used by the user to specify extra arguments for the host
> > compiler.
> >
> Do'h... the original patch was against -rc1 and changed both
> HOST_EXTRACFLAGS and HOST_LOADLIBES, but I cherry-picked it to be
> against your `for-next' and more particularly commit 7080e47bb, which
> removes the assignation of HOST_LOADLIBES. As I didn't pay attention
> and kept the plural :/

I changed it to "this".

> Anyway, with this patch and specifying HOST_EXTRACFLAGS in the
> environment rather than as a gmake(1) argument, it is properly
> appended. nconf links and runs fine on NetBSD. It should not impact
> other environment.

OK, applied.

Michal

Re: [PATCH] kbuild: don't overwrite HOST_EXTRACFLAGS

[Sam Ravnborg]

On Thu, Aug 26, 2010 at 02:00:41PM +0200, Michal Marek wrote:
> On Sun, Aug 22, 2010 at 08:17:21PM -0400, Arnaud Lacombe wrote:
> > Hi,
> > 
> > On Sun, Aug 22, 2010 at 8:03 PM, Arnaud Lacombe <lacombar@gmail.com> wrote:
> > > These might be used by the user to specify extra arguments for the host
> > > compiler.
> > >
> > Do'h... the original patch was against -rc1 and changed both
> > HOST_EXTRACFLAGS and HOST_LOADLIBES, but I cherry-picked it to be
> > against your `for-next' and more particularly commit 7080e47bb, which
> > removes the assignation of HOST_LOADLIBES. As I didn't pay attention
> > and kept the plural :/
> 
> I changed it to "this".
> 
> > Anyway, with this patch and specifying HOST_EXTRACFLAGS in the
> > environment rather than as a gmake(1) argument, it is properly
> > appended. nconf links and runs fine on NetBSD. It should not impact
> > other environment.
> 
> OK, applied.

I do not mind the patch in question.
But do we really want to allow users to override HOST_EXTRACFLAGS?

For the builtin stuff we have a lot of options
to override stuff:

CFLAGS_MODULE, AFLAGS_MODULE, LDFLAGS_MODULE
CFLAGS_KERNEL, AFLAGS_KERNEL
KAFLAGS, KCFLAGS, KCPPFLAGS

Maybe we should formalize it so we give users a _documented_ way to
add additional options to the host CC?

As per above this could be: HOSTCFLAGS

Patch is simple - but what do you think?

	Sam

Re: [PATCH 1/4] kbuild: don't assign `stdout' and `stderr'

[Arnaud Lacombe]

Hi Michal,

On Tue, Aug 17, 2010 at 4:27 AM, Michal Marek <mmarek@suse.cz> wrote:
> On Mon, Aug 16, 2010 at 12:19:03AM -0400, Arnaud Lacombe wrote:
>> These may not be modifiable, as per C99 comment:
>>
>> section 7.19.5.4, note 232:
>> "
>> The primary use of the freopen function is to change the file associated with a
>> standard text stream (stderr, stdin, or stdout), as those identifiers need not
>> be modifiable lvalues to which the value returned by the fopen function may be
>> assigned.
>> "
>>
>> Use a duplicate of stdout as ncurses' output terminal, then redirect it
>> to /dev/null for conf_write().
>
> I would much rather get rid of this redirection completely. The config
> library should not play with stdout, but let the frontends display
> messages as they wish. This patch does it for nconfig. Nir, what do you
> think?
>
any news about this ? AFAICS, it not in `kbuild/kconfig'.

Thanks,
 - Arnaud

>
> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> index dc11d51..c07060a 100644
> --- a/scripts/kconfig/confdata.c
> +++ b/scripts/kconfig/confdata.c
> @@ -19,6 +19,9 @@
>  static void conf_warning(const char *fmt, ...)
>        __attribute__ ((format (printf, 1, 2)));
>
> +static void conf_message(const char *fmt, ...)
> +       __attribute__ ((format (printf, 1, 2)));
> +
>  static const char *conf_filename;
>  static int conf_lineno, conf_warnings, conf_unsaved;
>
> @@ -35,6 +38,29 @@ static void conf_warning(const char *fmt, ...)
>        conf_warnings++;
>  }
>
> +static void conf_default_message_callback(const char *fmt, va_list ap)
> +{
> +       printf("#\n# ");
> +       vprintf(fmt, ap);
> +       printf("\n#\n");
> +}
> +
> +static void (*conf_message_callback) (const char *fmt, va_list ap) =
> +       conf_default_message_callback;
> +void conf_set_message_callback(void (*fn) (const char *fmt, va_list ap))
> +{
> +       conf_message_callback = fn;
> +}
> +
> +static void conf_message(const char *fmt, ...)
> +{
> +       va_list ap;
> +
> +       va_start(ap, fmt);
> +       if (conf_message_callback)
> +               conf_message_callback(fmt, ap);
> +}
> +
>  const char *conf_get_configname(void)
>  {
>        char *name = getenv("KCONFIG_CONFIG");
> @@ -184,9 +210,8 @@ int conf_read_simple(const char *name, int def)
>                        name = conf_expand_value(prop->expr->left.sym->name);
>                        in = zconf_fopen(name);
>                        if (in) {
> -                               printf(_("#\n"
> -                                        "# using defaults found in %s\n"
> -                                        "#\n"), name);
> +                               conf_message(_("using defaults found in %s"),
> +                                        name);
>                                goto load;
>                        }
>                }
> @@ -651,9 +676,7 @@ next:
>                        return 1;
>        }
>
> -       printf(_("#\n"
> -                "# configuration written to %s\n"
> -                "#\n"), newname);
> +       conf_message(_("configuration written to %s"), newname);
>
>        sym_set_change_count(0);
>
> diff --git a/scripts/kconfig/lkc_proto.h b/scripts/kconfig/lkc_proto.h
> index 9a948c9..481d484 100644
> --- a/scripts/kconfig/lkc_proto.h
> +++ b/scripts/kconfig/lkc_proto.h
> @@ -1,3 +1,4 @@
> +#include <stdarg.h>
>
>  /* confdata.c */
>  P(conf_parse,void,(const char *name));
> @@ -8,6 +9,7 @@ P(conf_write,int,(const char *name));
>  P(conf_write_autoconf,int,(void));
>  P(conf_get_changed,bool,(void));
>  P(conf_set_changed_callback, void,(void (*fn)(void)));
> +P(conf_set_message_callback, void,(void (*fn)(const char *fmt, va_list ap)));
>
>  /* menu.c */
>  P(rootmenu,struct menu,);
> diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
> index 18a215d..16233a9 100644
> --- a/scripts/kconfig/nconf.c
> +++ b/scripts/kconfig/nconf.c
> @@ -651,25 +651,6 @@ static const char *set_config_filename(const char *config_filename)
>        return menu_backtitle;
>  }
>
> -/* command = 0 is supress, 1 is restore */
> -static void supress_stdout(int command)
> -{
> -       static FILE *org_stdout;
> -       static FILE *org_stderr;
> -
> -       if (command == 0) {
> -               org_stdout = stdout;
> -               org_stderr = stderr;
> -               stdout = fopen("/dev/null", "a");
> -               stderr = fopen("/dev/null", "a");
> -       } else {
> -               fclose(stdout);
> -               fclose(stderr);
> -               stdout = org_stdout;
> -               stderr = org_stderr;
> -       }
> -}
> -
>  /* return = 0 means we are successful.
>  * -1 means go on doing what you were doing
>  */
> @@ -695,9 +676,7 @@ static int do_exit(void)
>        /* if we got here, the user really wants to exit */
>        switch (res) {
>        case 0:
> -               supress_stdout(0);
>                res = conf_write(filename);
> -               supress_stdout(1);
>                if (res)
>                        btn_dialog(
>                                main_window,
> @@ -707,19 +686,6 @@ static int do_exit(void)
>                                  "changes were NOT saved."),
>                                  1,
>                                  "<OK>");
> -               else {
> -                       char buf[1024];
> -                       snprintf(buf, 1024,
> -                               _("Configuration written to %s\n"
> -                                 "End of Linux kernel configuration.\n"
> -                                 "Execute 'make' to build the kernel or try"
> -                                 " 'make help'."), filename);
> -                       btn_dialog(
> -                               main_window,
> -                               buf,
> -                               1,
> -                               "<OK>");
> -               }
>                break;
>        default:
>                btn_dialog(
> @@ -1255,6 +1221,14 @@ static void conf(struct menu *menu)
>        }
>  }
>
> +static void conf_message_callback(const char *fmt, va_list ap)
> +{
> +       char buf[1024];
> +
> +       vsnprintf(buf, sizeof(buf), fmt, ap);
> +       btn_dialog(main_window, buf, 1, "<OK>");
> +}
> +
>  static void show_help(struct menu *menu)
>  {
>        struct gstr help = str_new();
> @@ -1477,16 +1451,8 @@ static void conf_save(void)
>                case 0:
>                        if (!dialog_input_result[0])
>                                return;
> -                       supress_stdout(0);
>                        res = conf_write(dialog_input_result);
> -                       supress_stdout(1);
>                        if (!res) {
> -                               char buf[1024];
> -                               sprintf(buf, "%s %s",
> -                                       _("configuration file saved to: "),
> -                                       dialog_input_result);
> -                               btn_dialog(main_window,
> -                                          buf, 1, "<OK>");
>                                set_config_filename(dialog_input_result);
>                                return;
>                        }
> @@ -1579,6 +1545,7 @@ int main(int ac, char **av)
>                                _(menu_no_f_instructions));
>        }
>
> +       conf_set_message_callback(conf_message_callback);
>        /* do the work */
>        while (!global_exit) {
>                conf(&rootmenu);
>

Re: [PATCH 1/4] kbuild: don't assign `stdout' and `stderr'

[Michal Marek]

On 21.10.2010 04:22, Arnaud Lacombe wrote:
> Hi Michal,
> 
> On Tue, Aug 17, 2010 at 4:27 AM, Michal Marek <mmarek@suse.cz> wrote:
>> On Mon, Aug 16, 2010 at 12:19:03AM -0400, Arnaud Lacombe wrote:
>>> These may not be modifiable, as per C99 comment:
>>>
>>> section 7.19.5.4, note 232:
>>> "
>>> The primary use of the freopen function is to change the file associated with a
>>> standard text stream (stderr, stdin, or stdout), as those identifiers need not
>>> be modifiable lvalues to which the value returned by the fopen function may be
>>> assigned.
>>> "
>>>
>>> Use a duplicate of stdout as ncurses' output terminal, then redirect it
>>> to /dev/null for conf_write().
>>
>> I would much rather get rid of this redirection completely. The config
>> library should not play with stdout, but let the frontends display
>> messages as they wish. This patch does it for nconfig. Nir, what do you
>> think?
>>
> any news about this ? AFAICS, it not in `kbuild/kconfig'.

Sorry, I forgot about this patch. It's merged into the kconfig branch now.

Michal