Re: [PATCH] kconfig: handle SIGINT in menuconfig

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Sat, Aug 20, 2011 at 4:50 PM, Davidlohr Bueso <dave@xxxxxxx> wrote:
> From: Davidlohr Bueso <dave@xxxxxxx>
>
> I recently got bitten in the ass when pressing Ctrl-C and lost all my current configuration changes. This patch captures SIGINT and allows the user to save any changes.
Pretty much all front-ends have that behavior.

> Some code refactoring was made in order to handle the exit behavior.
>
beside a few nits, that look good.

 - Arnaud

> CC: Roman Zippel <zippel@xxxxxxxxxxxxxx>
Roman has been reported MIA for more than 2 years now, I do not think
this is needed anylonger.

> Signed-off-by: Davidlohr Bueso <dave@xxxxxxx>
> ---
>  scripts/kconfig/mconf.c |   76 ++++++++++++++++++++++++++++++++---------------
>  1 files changed, 52 insertions(+), 24 deletions(-)
>
> diff --git a/scripts/kconfig/mconf.c b/scripts/kconfig/mconf.c
> index 820d2b6..7ca3bb7 100644
> --- a/scripts/kconfig/mconf.c
> +++ b/scripts/kconfig/mconf.c
> @@ -6,6 +6,7 @@
>  * 2002-11-06 Petr Baudis <pasky@xxxxxx>
>  *
>  * i18n, 2005, Arnaldo Carvalho de Melo <acme@xxxxxxxxxxxxxxxx>
> + * Handle SIGINT (Ctrl-C), 2011, Davidlohr Bueso <dave@xxxxxxx>
>  */
>
FWIW, I do not really see the point of that, if you are looking for
code history git does a better job, as well as for who deserve
copyright on the code.

>  #include <ctype.h>
> @@ -15,6 +16,7 @@
>  #include <stdarg.h>
>  #include <stdlib.h>
>  #include <string.h>
> +#include <signal.h>
>  #include <unistd.h>
>  #include <locale.h>
>
> @@ -272,6 +274,7 @@ static struct menu *current_menu;
>  static int child_count;
>  static int single_menu_mode;
>  static int show_all_options;
> +static int saved_x, saved_y;
>
>  static void conf(struct menu *menu);
>  static void conf_choice(struct menu *menu);
> @@ -792,9 +795,54 @@ static void conf_save(void)
>        }
>  }
>
> +static int handle_exit(int res)
> +{
> +       switch (res) {
> +       case 0:
> +               if (conf_write(filename)) {
> +                       fprintf(stderr, _("\n\n"
> +                                         "Error while writing of the configuration.\n"
> +                                         "Your configuration changes were NOT saved."
> +                                         "\n\n"));
> +                       return 1;
> +               }
> +               /* fall through */
> +       case -1:
> +               printf(_("\n\n"
> +                        "*** End of the configuration.\n"
> +                        "*** Execute 'make' to start the build or try 'make help'."
> +                        "\n\n"));
> +               break;
> +       default:
> +               fprintf(stderr, _("\n\n"
> +                                 "Your configuration changes were NOT saved."
> +                                 "\n\n"));
> +       }
> +
> +       return 0;
> +}
> +
> +static void sig_handler(int signo __attribute__((__unused__)))
__attribute__(()) is useless here, it is not used once across the file
and gcc will not warn unless you ask it to be really verbose, which we
do not.

> +{
> +       int res;
> +
> +       do {
> +               dialog_clear();
> +               if (conf_get_changed())
> +                       res = dialog_yesno(NULL,
> +                                          _("Do you wish to save your "
> +                                            "new configuration?\n"),
> +                                          6, 60);
> +               else
> +                       res = -1;
> +       } while (res == KEY_ESC);
> +
I do not really see the point of the loop here.

I'd suggest to have a single termination handling path. How about the
attached patch ?

The message displayed might need tweaking.

Thanks,
 - Arnaud

> +       end_dialog(saved_x, saved_y);
> +       exit(handle_exit(res));
> +}
> +
>  int main(int ac, char **av)
>  {
> -       int saved_x, saved_y;
>        char *mode;
>        int res;
>
> @@ -802,6 +850,8 @@ int main(int ac, char **av)
>        bindtextdomain(PACKAGE, LOCALEDIR);
>        textdomain(PACKAGE);
>
> +       signal(SIGINT, sig_handler);
> +
>        conf_parse(av[1]);
>        conf_read(NULL);
>
> @@ -835,28 +885,6 @@ int main(int ac, char **av)
>        } while (res == KEY_ESC);
>        end_dialog(saved_x, saved_y);
>
> -       switch (res) {
> -       case 0:
> -               if (conf_write(filename)) {
> -                       fprintf(stderr, _("\n\n"
> -                               "Error while writing of the configuration.\n"
> -                               "Your configuration changes were NOT saved."
> -                               "\n\n"));
> -                       return 1;
> -               }
> -               /* fall through */
> -       case -1:
> -               printf(_("\n\n"
> -                       "*** End of the configuration.\n"
> -                       "*** Execute 'make' to start the build or try 'make help'."
> -                       "\n\n"));
> -               break;
> -       default:
> -               fprintf(stderr, _("\n\n"
> -                       "Your configuration changes were NOT saved."
> -                       "\n\n"));
> -       }
> -
> -       return 0;
> +       return handle_exit(res);
>  }
>
> --
> 1.7.4.1
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
diff --git a/scripts/kconfig/mconf.c b/scripts/kconfig/mconf.c
index 7ca3bb7..90c7b2e 100644
--- a/scripts/kconfig/mconf.c
+++ b/scripts/kconfig/mconf.c
@@ -795,8 +795,21 @@ static void conf_save(void)
 	}
 }
 
-static int handle_exit(int res)
+static int handle_exit(int fatal)
 {
+	int res;
+
+	dialog_clear();
+	if (conf_get_changed())
+		res = dialog_yesno(NULL,
+				   _("Do you wish to save your new configuration ?\n"
+				     "<ESC><ESC> to continue."),
+				   6, 60);
+	else
+		res = -1;
+
+	end_dialog(saved_x, saved_y);
+
 	switch (res) {
 	case 0:
 		if (conf_write(filename)) {
@@ -812,6 +825,7 @@ static int handle_exit(int res)
 			 "*** End of the configuration.\n"
 			 "*** Execute 'make' to start the build or try 'make help'."
 			 "\n\n"));
+		res = 0;
 		break;
 	default:
 		fprintf(stderr, _("\n\n"
@@ -819,26 +833,13 @@ static int handle_exit(int res)
 				  "\n\n"));
 	}
 
-	return 0;
+	return res;
 }
 
 static void sig_handler(int signo __attribute__((__unused__)))
 {
-	int res;
-	
-	do {
-		dialog_clear();
-		if (conf_get_changed())
-			res = dialog_yesno(NULL,
-					   _("Do you wish to save your "
-					     "new configuration?\n"),
-					   6, 60);
-		else
-			res = -1;
-	} while (res == KEY_ESC);
 
-	end_dialog(saved_x, saved_y);
-	exit(handle_exit(res));
+	exit(handle_exit(1));
 }
 
 int main(int ac, char **av)
@@ -873,18 +874,9 @@ int main(int ac, char **av)
 	set_config_filename(conf_get_configname());
 	do {
 		conf(&rootmenu);
-		dialog_clear();
-		if (conf_get_changed())
-			res = dialog_yesno(NULL,
-					   _("Do you wish to save your "
-					     "new configuration?\n"
-					     "<ESC><ESC> to continue."),
-					   6, 60);
-		else
-			res = -1;
+		res = handle_exit(0);
 	} while (res == KEY_ESC);
-	end_dialog(saved_x, saved_y);
 
-	return handle_exit(res);
+	return res;
 }
 

[Index of Archives]     [Linux&nblp;USB Development]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite Secrets]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux