kbuild: don't assign `stdout' and `stderr'

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 <[email protected]>

---
 scripts/kconfig/Makefile |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

scripts/kconfig/Makefile HOSTCFLAGS_gconf.o = `pkg-config --cflags gtk+-2.0 gmodule-2.0 libglade-2.0` \ · line 230

Quoted 2 lines

scripts/kconfig/Makefile +1 -1

-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?

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

Hi,

Quoted 3 lines · Arnaud Lacombe

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.

Quoted 11 lines · Arnaud Lacombe

Hi,

Quoted 3 lines · Arnaud Lacombe

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".

Quoted 4 lines

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.

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().

Quoted 10 lines · 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.
"

Quoted 2 lines

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.

Quoted 10 lines · 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.
"

Quoted 2 lines

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?

scripts/kconfig/confdata.c +29 -6

index dc11d51..c07060a 100644
@@ -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);
 

scripts/kconfig/lkc_proto.h +2 -0

index 9a948c9..481d484 100644
@@ -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,);

scripts/kconfig/nconf.c +0 -19

index 18a215d..16233a9 100644
@@ -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,

scripts/kconfig/nconf.c +0 -13

@@ -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(

scripts/kconfig/nconf.c +8 -0

@@ -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;
                       }

Quoted 10 lines · 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.
"

Quoted 2 lines

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?

scripts/kconfig/confdata.c +29 -6

index dc11d51..c07060a 100644
@@ -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);
 

scripts/kconfig/lkc_proto.h +2 -0

index 9a948c9..481d484 100644
@@ -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,);

scripts/kconfig/nconf.c +0 -19

index 18a215d..16233a9 100644
@@ -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,

scripts/kconfig/nconf.c +0 -13

@@ -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(

scripts/kconfig/nconf.c +8 -0

@@ -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;
                       }

scripts/kconfig/nconf.c +1 -0

@@ -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 Michal,

Quoted 14 lines · Michal Marek

Quoted 12 lines · 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().

Quoted 5 lines

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'.