Re: [PATCH 1/2] hello: Mark print_help() not to return

[Mathieu Lirzin]

Hello Sami,

Sami Kerola <address@hidden> writes:

> * bootstrap.conf: include stdnoreturn module.
> * src/hello.c: mark print_help() not to return, and remove unreachable
>   code.
>
> Reference: https://www.gnu.org/software/gnulib/manual/html_node/
> stdnoreturn_002eh.html
> ---
>  bootstrap.conf |  1 +
>  src/hello.c    | 10 +++++-----
>  2 files changed, 6 insertions(+), 5 deletions(-)
>
[...]
> diff --git a/src/hello.c b/src/hello.c
> index 8e6cfbd..55edfa4 100644
> --- a/src/hello.c
> +++ b/src/hello.c
> @@ -18,6 +18,8 @@
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>  
>  #include <config.h>
> +#include <stdnoreturn.h>
> +
>  #include "system.h"
>  #include "errno.h"
>  #include "error.h"
> @@ -25,7 +27,7 @@
>  #include "xalloc.h"
>  
>  /* Forward declarations.  */
> -static void print_help (FILE *out);
> +static _Noreturn void print_help (FILE *restrict out);
>  static void print_version (void);
>  
>  int
> @@ -82,8 +84,6 @@ main (int argc, char *argv[])
>       break;
>        case OPT_HELP:
>       print_help (stdout);
> -     exit (EXIT_SUCCESS);
> -     break;

IMHO, calling 'exit' inside functions makes the code harder to
understand and should be avoid when possible.  As a consequence I would
prefer GNU Hello not to implicitly advertise its usage.

>        case 't':
>       greeting_msg = _("hello, world");
>       break;
> @@ -117,8 +117,8 @@ main (int argc, char *argv[])
>     several pieces to help translators be able to align different
>     blocks and identify the various pieces.  */
>  
> -static void
> -print_help (FILE *out)
> +static _Noreturn void
> +print_help (FILE *restrict out)
>  {
>    const char *lc_messages = setlocale (LC_MESSAGES, NULL);
>    /* TRANSLATORS: --help output 1 (synopsis)

What about removing the last line in the 'print_help' function:

  exit(out == stderr ? EXIT_FAILURE : EXIT_SUCCESS);

and moving the remaining 'exit' call inside the 'main' function?

WDYT?

-- 
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761  070D 0ADE E100 9460 4D37