From 7ad98986ffbd56060ded51698abd7268238c3120 Mon Sep 17 00:00:00 2001 From: James Youngman Date: Fri, 2 Apr 2010 17:47:39 +0100 Subject: [PATCH] Avoid using magic numbers as exit statuses. Avoid using magic numbers as exit statuses. * cfg.mk (skip_defer): Enable sc_prohibit_magic_number_exit. * doc/find-maint.texi (Security): Avoid error (1, ...) in the example patch. * find/find.c (main): Use exit status EXIT_FAILURE instead of 1. (wd_sanity_check): Likewise. (safely_chdir_lstat): Likewise. (process_dir): Likewise. * find/fstype.c (must_read_fs_list): Likewise. * find/ftsfind.c (main): Likewise. (main): Likewise. * find/parser.c (check_option_combinations): Use exit status EXIT_FAILURE instead of 1. (parse_group): Likewise. (parse_help): Use exit status EXIT_SUCCESS instead of 0. (parse_version): Likewise. (fnmatch_sanitycheck): Use exit status EXIT_FAILURE instead of 1. (insert_depthspec): Likewise. (parse_newerXY): Likewise. (non_posix_mode): Likewise. (parse_perm): Likewise. (insert_regex): Likewise. (parse_size): Likewise. (parse_show_control_chars): Likewise. (parse_used): Likewise. (parse_user): Likewise. (parse_context): Likewise. (insert_type): Likewise. (insert_fprintf): Likewise. (make_segment): Likewise. (check_path_safety): Likewise. (new_insert_exec_ok): Likewise. (get_relative_timestamp): Likewise. (parse_time): Likewise. * find/tree.c (get_expr): Use exit status EXIT_FAILURE instead of 1. (scan_rest): Likewise. (get_pred_cost): Likewise. (get_pred_cost): Likewise. (build_expression_tree): Likewise. (get_new_pred_chk_op): Likewise. * find/pred.c (launch): Use exit status EXIT_FAILURE instead of 1. * find/util.c (process_debug_options): Use exit status EXIT_SUCCESS instead of 0. (process_optimisation_option): Use exit status EXIT_FAILURE instead of 1. (set_option_defaults): Likewise. * lib/buildcmd.c (exceeds): Likewise. (bc_do_exec): Likewise. (bc_push_arg): Likewise. * lib/nextelem.c: Remove test code. * lib/safe-atoi.c (safe_atoi): Use exit status EXIT_FAILURE instead of 1. * lib/regextype.c (get_regex_type): Use exit status EXIT_FAILURE instead of 1. Include , for EXIT_FAILURE. * locate/locate.c (set_max_db_age): Use exit status EXIT_FAILURE instead of 1. (set_max_db_age): Likewise. (toolong): Likewise. (visit_locate02_format): Likewise. (search_one_database): Likewise. (drop_privs): Likewise. * locate/frcode.c (get_seclevel): Use exit status EXIT_FAILURE instead of 1. (outerr): Likewise. (main): Likewise. * locate/code.c (inerr): Use exit status EXIT_FAILURE instead of 1. (outerr): Likewise. * locate/word_io.c (getword): Use exit status EXIT_FAILURE instead of 1. Use named constants for xargs exit codes. * xargs/xargs.c (child_error): Use EXIT_SUCCESS instead of 0. (enum XargsStatusValues): introduce named constants for the various things that cause speicific exit codes. (enum ClientStatusValues): Also introduce named constants for the child's exit status. (get_char_oct_or_hex_escape): Use EXIT_FAILURE instead of 1. (get_input_delimiter): Likewise. (main): Likewise. (read_line): Likewise. (read_string): Likewise. (print_args): Likewise. (xargs_do_exec): Likewise. (wait_for_proc): Likewise. (wait_for_proc): Use enum XargsStatusValues for exit codes. (parse_num): Use EXIT_FAILURE instead of 1. Signed-off-by: James Youngman --- ChangeLog | 87 +++++++++++++++++++++++++++++++++++++++++++++ cfg.mk | 2 +- doc/find-maint.texi | 2 +- find/parser.c | 6 ++-- find/tree.c | 1 + find/util.c | 2 +- lib/buildcmd.c | 1 + lib/nextelem.c | 13 ------- lib/regextype.c | 1 + lib/safe-atoi.c | 10 +++--- xargs/xargs.c | 48 +++++++++++++++++-------- 11 files changed, 135 insertions(+), 38 deletions(-) diff --git a/ChangeLog b/ChangeLog index 39221c13..b9673125 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,92 @@ 2010-04-02 James Youngman + Use named constants for xargs exit codes. + * xargs/xargs.c (child_error): Use EXIT_SUCCESS instead of 0. + (enum XargsStatusValues): introduce named constants for the + various things that cause speicific exit codes. + (enum ClientStatusValues): Also introduce named constants for the + child's exit status. + (get_char_oct_or_hex_escape): Use EXIT_FAILURE instead of 1. + (get_input_delimiter): Likewise. + (main): Likewise. + (read_line): Likewise. + (read_string): Likewise. + (print_args): Likewise. + (xargs_do_exec): Likewise. + (wait_for_proc): Likewise. + (wait_for_proc): Use enum XargsStatusValues for exit codes. + (parse_num): Use EXIT_FAILURE instead of 1. + + Avoid using magic numbers as exit statuses. + * cfg.mk (skip_defer): Enable sc_prohibit_magic_number_exit. + * doc/find-maint.texi (Security): Avoid error (1, ...) in the + example patch. + * find/find.c (main): Use exit status EXIT_FAILURE instead of 1. + (wd_sanity_check): Likewise. + (safely_chdir_lstat): Likewise. + (process_dir): Likewise. + * find/fstype.c (must_read_fs_list): Likewise. + * find/ftsfind.c (main): Likewise. + (main): Likewise. + * find/parser.c (check_option_combinations): Use exit status + EXIT_FAILURE instead of 1. + (parse_group): Likewise. + (parse_help): Use exit status EXIT_SUCCESS instead of 0. + (parse_version): Likewise. + (fnmatch_sanitycheck): Use exit status EXIT_FAILURE instead of 1. + (insert_depthspec): Likewise. + (parse_newerXY): Likewise. + (non_posix_mode): Likewise. + (parse_perm): Likewise. + (insert_regex): Likewise. + (parse_size): Likewise. + (parse_show_control_chars): Likewise. + (parse_used): Likewise. + (parse_user): Likewise. + (parse_context): Likewise. + (insert_type): Likewise. + (insert_fprintf): Likewise. + (make_segment): Likewise. + (check_path_safety): Likewise. + (new_insert_exec_ok): Likewise. + (get_relative_timestamp): Likewise. + (parse_time): Likewise. + * find/tree.c (get_expr): Use exit status EXIT_FAILURE instead of 1. + (scan_rest): Likewise. + (get_pred_cost): Likewise. + (get_pred_cost): Likewise. + (build_expression_tree): Likewise. + (get_new_pred_chk_op): Likewise. + * find/pred.c (launch): Use exit status EXIT_FAILURE instead of 1. + * find/util.c (process_debug_options): Use exit status + EXIT_SUCCESS instead of 0. + (process_optimisation_option): Use exit status EXIT_FAILURE + instead of 1. + (set_option_defaults): Likewise. + * lib/buildcmd.c (exceeds): Likewise. + (bc_do_exec): Likewise. + (bc_push_arg): Likewise. + * lib/nextelem.c: Remove test code. + * lib/safe-atoi.c (safe_atoi): Use exit status EXIT_FAILURE + instead of 1. + * lib/regextype.c (get_regex_type): Use exit status EXIT_FAILURE + instead of 1. Include , for EXIT_FAILURE. + * locate/locate.c (set_max_db_age): Use exit status EXIT_FAILURE + instead of 1. + (set_max_db_age): Likewise. + (toolong): Likewise. + (visit_locate02_format): Likewise. + (search_one_database): Likewise. + (drop_privs): Likewise. + * locate/frcode.c (get_seclevel): Use exit status EXIT_FAILURE + instead of 1. + (outerr): Likewise. + (main): Likewise. + * locate/code.c (inerr): Use exit status EXIT_FAILURE instead of + 1. + (outerr): Likewise. + * locate/word_io.c (getword): Use exit status EXIT_FAILURE instead of 1. + Avoid sc_prohibit_stat_st_blocks check, since it is broken. * cfg.mk (skip_broken_checks): Skip sc_prohibit_stat_st_blocks. diff --git a/cfg.mk b/cfg.mk index ef981d5c..46689fb8 100644 --- a/cfg.mk +++ b/cfg.mk @@ -23,7 +23,7 @@ skip_dunno = sc_immutable_NEWS sc_makefile_at_at_check \ sc_prohibit_quote_without_use sc_prohibit_quotearg_without_use # Understand, but fix later. -skip_defer = sc_program_name sc_prohibit_magic_number_exit +skip_defer = sc_program_name # False positives I don't have a workaround for yet. # sc_space_tab: several .xo test output files contain this sequence diff --git a/doc/find-maint.texi b/doc/find-maint.texi index 4f4dc0f2..38b88cad 100644 --- a/doc/find-maint.texi +++ b/doc/find-maint.texi @@ -987,7 +987,7 @@ diff -u -p -r1.58.2.2 locate.c +static void +toolong (struct process_data *procdata) +@{ -+ error (1, 0, ++ error (EXIT_FAILURE, 0, + _("locate database %s contains a " + "filename longer than locate can handle"), + procdata->dbfile); diff --git a/find/parser.c b/find/parser.c index 3406cd63..2fa302c1 100644 --- a/find/parser.c +++ b/find/parser.c @@ -1258,7 +1258,7 @@ actions: -delete -print0 -printf FORMAT -fprintf FILE FORMAT -print \n\ puts (_("Report (and track progress on fixing) bugs via the findutils bug-reporting\n\ page at http://savannah.gnu.org/ or, if you have no web access, by sending\n\ email to .")); - exit (0); + exit (EXIT_SUCCESS); } static float @@ -2759,7 +2759,7 @@ parse_version (const struct parser_table* entry, char **argv, int *arg_ptr) } printf ("\n"); - exit (0); + exit (EXIT_SUCCESS); } static boolean @@ -3259,7 +3259,7 @@ check_path_safety (const char *action, char **argv) "Please remove the current directory from your " "$PATH (that is, remove \".\" or leading or trailing " "colons)"), - action); + action); } else if ('/' != s[0]) { diff --git a/find/tree.c b/find/tree.c index 5238e90e..08f2f0a9 100644 --- a/find/tree.c +++ b/find/tree.c @@ -196,6 +196,7 @@ get_expr (struct predicate **input, || ((*input)->p_type != CLOSE_PAREN)) error (EXIT_FAILURE, 0, _("invalid expression; I was expecting to find a ')' somewhere but did not see one.")); + *input = (*input)->pred_next; /* move over close */ break; diff --git a/find/util.c b/find/util.c index 5a62fb19..9294f317 100644 --- a/find/util.c +++ b/find/util.c @@ -817,7 +817,7 @@ process_debug_options (char *arg) else if (options.debug_options & DebugHelp) { show_valid_debug_options (stdout, 1); - exit (0); + exit (EXIT_SUCCESS); } } diff --git a/lib/buildcmd.c b/lib/buildcmd.c index 901f381f..23fa528b 100644 --- a/lib/buildcmd.c +++ b/lib/buildcmd.c @@ -385,6 +385,7 @@ bc_push_arg (struct buildcmd_control *ctl, if (initial_args || state->cmd_argc == ctl->initial_argc) error (EXIT_FAILURE, 0, _("can not fit single argument within argument list size limit")); + /* xargs option -i (replace_pat) implies -x (exit_if_size_exceeded) */ if (ctl->replace_pat || (ctl->exit_if_size_exceeded && diff --git a/lib/nextelem.c b/lib/nextelem.c index 2d42de01..c370c380 100644 --- a/lib/nextelem.c +++ b/lib/nextelem.c @@ -83,16 +83,3 @@ next_element (const char *new_path, int curdir_ok) return start; } - -#ifdef TEST -int -main () -{ - char *p; - - next_element (getenv ("PATH")); - while (p = next_element (NULL)) - puts (p); - exit (0); -} -#endif /* TEST */ diff --git a/lib/regextype.c b/lib/regextype.c index 055218f5..5b391dd0 100644 --- a/lib/regextype.c +++ b/lib/regextype.c @@ -23,6 +23,7 @@ #include #include +#include #include #include "regextype.h" diff --git a/lib/safe-atoi.c b/lib/safe-atoi.c index 235d0e98..296ee3bd 100644 --- a/lib/safe-atoi.c +++ b/lib/safe-atoi.c @@ -53,12 +53,12 @@ safe_atoi (const char *s, enum quoting_style style) if (errno == ERANGE) { /* too big, or too small. */ - error (1, errno, "%s", s); + error (EXIT_FAILURE, errno, "%s", s); } else { /* not a valid number */ - error (1, errno, "%s", s); + error (EXIT_FAILURE, errno, "%s", s); } /* Otherwise, we do a range chack against INT_MAX and INT_MIN * below. @@ -69,17 +69,17 @@ safe_atoi (const char *s, enum quoting_style style) { /* The number was in range for long, but not int. */ errno = ERANGE; - error (1, errno, "%s", s); + error (EXIT_FAILURE, errno, "%s", s); } else if (*end) { - error (1, errno, _("Unexpected suffix %s on %s"), + error (EXIT_FAILURE, errno, _("Unexpected suffix %s on %s"), quotearg_n_style (0, style, end), quotearg_n_style (1, style, s)); } else if (end == s) { - error (1, errno, _("Expected an integer: %s"), + error (EXIT_FAILURE, errno, _("Expected an integer: %s"), quotearg_n_style (0, style, s)); } return (int)lval; diff --git a/xargs/xargs.c b/xargs/xargs.c index d8a0015f..a4145c51 100644 --- a/xargs/xargs.c +++ b/xargs/xargs.c @@ -168,7 +168,7 @@ static pid_t parent; /* Exit status; nonzero if any child process exited with a status of 1-125. */ -static volatile int child_error = 0; +static volatile int child_error = EXIT_SUCCESS; static volatile int original_exit_value; @@ -206,6 +206,23 @@ static struct option const longopts[] = {NULL, no_argument, NULL, 0} }; +/* xargs exits with status values with specific meanings (this is a POSIX requirement). + These are the values. +*/ +enum XargsStatusValues { + XARGS_EXIT_CLIENT_EXIT_NONZERO = 123, /* utility exited with nonzero status */ + XARGS_EXIT_CLIENT_EXIT_255 = 124, /* utility exites with status 255 */ + XARGS_EXIT_CLIENT_FATAL_SIG = 125, /* utility died from a fatal signal */ + XARGS_EXIT_COMMAND_CANNOT_BE_RUN = 126, /* canot run the command */ + XARGS_EXIT_COMMAND_NOT_FOUND = 127, /* cannot find the command */ +}; +/* Exit status values the child might use. */ +enum ClientStatusValues { + CHILD_EXIT_PLEASE_STOP_IMMEDIATELY = 255 +}; + + + static int read_line PARAMS ((void)); static int read_string PARAMS ((void)); static boolean print_args PARAMS ((boolean ask)); @@ -370,7 +387,7 @@ main (int argc, char **argv) program_name = argv[0]; parent = getpid (); - original_exit_value = 0; + original_exit_value = EXIT_SUCCESS; #ifdef HAVE_SETLOCALE setlocale (LC_ALL, ""); @@ -1093,7 +1110,7 @@ xargs_do_exec (struct buildcmd_control *ctl, void *usercontext, int argc, char * case 0: /* Child. */ { close (fd[0]); - child_error = 0; + child_error = EXIT_SUCCESS; prep_child_for_exec (); @@ -1126,7 +1143,7 @@ xargs_do_exec (struct buildcmd_control *ctl, void *usercontext, int argc, char * /* The actual value returned here should be irrelevant, * because the parent will test our value of errno. */ - _exit (errno == ENOENT ? 127 : 126); + _exit (errno == ENOENT ? XARGS_EXIT_COMMAND_NOT_FOUND : XARGS_EXIT_COMMAND_CANNOT_BE_RUN); /*NOTREACHED*/ } /* child */ @@ -1176,11 +1193,11 @@ xargs_do_exec (struct buildcmd_control *ctl, void *usercontext, int argc, char * } else if (ENOENT == buf) { - exit (127); /* command cannot be found */ + exit (XARGS_EXIT_COMMAND_NOT_FOUND); /* command cannot be found */ } else { - exit (126); /* command cannot be run */ + exit (XARGS_EXIT_COMMAND_CANNOT_BE_RUN); /* command cannot be run */ } break; } @@ -1328,14 +1345,17 @@ wait_for_proc (boolean all, unsigned int minreap) procs_executing--; reaped++; - if (WEXITSTATUS (status) == 255) - error (124, 0, _("%s: exited with status 255; aborting"), bc_state.cmd_argv[0]); + if (WEXITSTATUS (status) == CHILD_EXIT_PLEASE_STOP_IMMEDIATELY) + error (XARGS_EXIT_CLIENT_EXIT_255, 0, + _("%s: exited with status 255; aborting"), bc_state.cmd_argv[0]); if (WIFSTOPPED (status)) - error (125, 0, _("%s: stopped by signal %d"), bc_state.cmd_argv[0], WSTOPSIG (status)); + error (XARGS_EXIT_CLIENT_FATAL_SIG, 0, + _("%s: stopped by signal %d"), bc_state.cmd_argv[0], WSTOPSIG (status)); if (WIFSIGNALED (status)) - error (125, 0, _("%s: terminated by signal %d"), bc_state.cmd_argv[0], WTERMSIG (status)); + error (XARGS_EXIT_CLIENT_FATAL_SIG, 0, + _("%s: terminated by signal %d"), bc_state.cmd_argv[0], WTERMSIG (status)); if (WEXITSTATUS (status) != 0) - child_error = 123; + child_error = XARGS_EXIT_CLIENT_EXIT_NONZERO; } } @@ -1394,7 +1414,7 @@ parse_num (char *str, int option, long int min, long int max, int fatal) fprintf (stderr, _("%s: invalid number for -%c option\n"), program_name, option); usage (stderr); - exit (1); + exit (EXIT_FAILURE); } else if (val < min) { @@ -1403,7 +1423,7 @@ parse_num (char *str, int option, long int min, long int max, int fatal) if (fatal) { usage (stderr); - exit (1); + exit (EXIT_FAILURE); } else { @@ -1417,7 +1437,7 @@ parse_num (char *str, int option, long int min, long int max, int fatal) if (fatal) { usage (stderr); - exit (1); + exit (EXIT_FAILURE); } else {