From patchwork Sat Aug 20 22:12:05 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Arnaud Lacombe X-Patchwork-Id: 1083012 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter2.kernel.org (8.14.4/8.14.4) with ESMTP id p7KMCFj9013630 for ; Sat, 20 Aug 2011 22:12:15 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755152Ab1HTWMH (ORCPT ); Sat, 20 Aug 2011 18:12:07 -0400 Received: from mail-pz0-f42.google.com ([209.85.210.42]:58291 "EHLO mail-pz0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755134Ab1HTWMG (ORCPT ); Sat, 20 Aug 2011 18:12:06 -0400 Received: by pzk37 with SMTP id 37so6737704pzk.1 for ; Sat, 20 Aug 2011 15:12:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=RvK06n4QXqGBGCVgjisQvmt79Y9frR/Bo/M6yxOBmos=; b=ubbHk1byoZyCXEvkgYtv9owG9U4JRoCB+idlpNaiudtP3wliTJxqz4De4mhq/nyqE4 x1PzHFjN5akgceG8iPgbZb0aCjKWwZyy/u5LtvHdGMss31sBAQmbvfc+Wq6Oje903ldw NDfw6D1Nw8KzavaKTlGjlksFFdvccn4xTXiac= MIME-Version: 1.0 Received: by 10.142.196.17 with SMTP id t17mr576537wff.276.1313878325259; Sat, 20 Aug 2011 15:12:05 -0700 (PDT) Received: by 10.68.49.101 with HTTP; Sat, 20 Aug 2011 15:12:05 -0700 (PDT) In-Reply-To: <1313873432.2655.1.camel@offbook> References: <1313873432.2655.1.camel@offbook> Date: Sat, 20 Aug 2011 18:12:05 -0400 Message-ID: Subject: Re: [PATCH] kconfig: handle SIGINT in menuconfig From: Arnaud Lacombe To: dave@gnu.org Cc: Michal Marek , lkml , linux-kbuild@vger.kernel.org Sender: linux-kbuild-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kbuild@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter2.kernel.org [140.211.167.43]); Sat, 20 Aug 2011 22:12:15 +0000 (UTC) Hi, On Sat, Aug 20, 2011 at 4:50 PM, Davidlohr Bueso wrote: > From: Davidlohr Bueso > > 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 Roman has been reported MIA for more than 2 years now, I do not think this is needed anylonger. > Signed-off-by: Davidlohr Bueso > --- >  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 >  * >  * i18n, 2005, Arnaldo Carvalho de Melo > + * Handle SIGINT (Ctrl-C), 2011, Davidlohr Bueso >  */ > 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 > @@ -15,6 +16,7 @@ >  #include >  #include >  #include > +#include >  #include >  #include > > @@ -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@vger.kernel.org > 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" + " 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" - " 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; }